[PATCH] D30427: Fix whitespace before token-paste of an argument.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.

The whitespace should come from the argument name in the macro
expansion, rather than from the token passed to the macro (same as it
does when not pasting).

Added a new test case for the change in behavior to stringize_space.c.

FileCheck'ized macro_paste_commaext.c, tweaked the test case, and
added a comment; no behavioral change to this test.


https://reviews.llvm.org/D30427

Files:
  lib/Lex/TokenLexer.cpp
  test/Preprocessor/macro_paste_commaext.c
  test/Preprocessor/stringize_space.c

Index: test/Preprocessor/stringize_space.c
===
--- test/Preprocessor/stringize_space.c
+++ test/Preprocessor/stringize_space.c
@@ -18,3 +18,14 @@
 1)
 
 // CHECK: {{^}}"-1"
+
+#define paste(a,b) str(a ".foo" not ". foo".
 if (i != 0 && !Tokens[i-1].is(tok::hashhash) && CurTok.hasLeadingSpace())
   NextTokGetsSpace = true;
 
@@ -317,14 +323,16 @@
 const Token *ArgToks = ActualArgs->getUnexpArgument(ArgNo);
 unsigned NumToks = MacroArgs::getArgLength(ArgToks);
 if (NumToks) {  // Not an empty argument?
+  bool VaArgsPseudoPaste = false;
   // If this is the GNU ", ## __VA_ARGS__" extension, and we just learned
   // that __VA_ARGS__ expands to multiple tokens, avoid a pasting error when
   // the expander trys to paste ',' with the first token of the __VA_ARGS__
   // expansion.
   if (NonEmptyPasteBefore && ResultToks.size() >= 2 &&
   ResultToks[ResultToks.size()-2].is(tok::comma) &&
   (unsigned)ArgNo == Macro->getNumArgs()-1 &&
   Macro->isVariadic()) {
+VaArgsPseudoPaste = true;
 // Remove the paste operator, report use of the extension.
 PP.Diag(ResultToks.pop_back_val().getLocation(), diag::ext_paste_comma);
   }
@@ -344,18 +352,16 @@
ResultToks.end()-NumToks, ResultToks.end());
   }
 
-  // If this token (the macro argument) was supposed to get leading
-  // whitespace, transfer this information onto the first token of the
-  // expansion.
-  //
-  // Do not do this if the paste operator occurs before the macro argument,
-  // as in "A ## MACROARG".  In valid code, the first token will get
-  // smooshed onto the preceding one anyway (forming AMACROARG).  In
-  // assembler-with-cpp mode, invalid pastes are allowed through: in this
-  // case, we do not want the extra whitespace to be added.  For example,
-  // we want ". ## foo" -> ".foo" not ". foo".
-  if (NextTokGetsSpace)
-ResultToks[ResultToks.size()-NumToks].setFlag(Token::LeadingSpace);
+  // Transfer the leading whitespace information from the token
+  // (the macro argument) onto the first token of the
+  // expansion. Note that we don't do this for the GNU
+  // pseudo-paste extension ", ## __VA_ARGS__".
+  if (!VaArgsPseudoPaste) {
+ResultToks[ResultToks.size()-NumToks].setFlagValue(Token::StartOfLine,
+   

[PATCH] D24688: Introduce "hosted" module flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Since this requires everyone generating llvm IR to add this module attribute 
for optimal codegen, it should be documented in release notes and the LangRef, 
I think.

I mean: it's unfortunate that it's needed at all, but at the very least it 
should be possible for people to find out about it.


https://reviews.llvm.org/D24688



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

[PATCH] D30427: Fix whitespace before token-paste of an argument.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ping.


https://reviews.llvm.org/D30427



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

[PATCH] D27387: [libc++] Add a key function for bad_function_call

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Since this is only adding a new export to libc++, not changing/breaking 
existing ones, I don't think it should be grouped with the ABI-breaking changes 
in v2.

The usual promise is that upgrading libc++.so.1 will not break apps compiled 
against an older set of headers, not that you're able to compile against a new 
set of headers and then run on an old libc++.so.1.

That is, even if the use of the new export is put under a #ifdef, it seems like 
it ought to default "on".


https://reviews.llvm.org/D27387



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

[PATCH] D31022: Implement P0298R3: `std::byte`

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I believe this needs compiler support, too, in order to treat

  namespace std { enum class byte : unsigned char {}; }

as directly having tbaa type "omnipotent char" instead of a subtype.

That is, given:

  void foo(char* x, int *y) {
x[1] = char(y[0] & 0xff);
x[0] = char((y[0] & 0xff00) >> 8);
  }

the compiler assumes that x and y might alias each-other, and thus must have 
two loads of y[0]. If you replace "char" with "std::byte", the same should 
happen, but as of now does not.


https://reviews.llvm.org/D31022



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

[PATCH] D30427: Fix whitespace before token-paste of an argument.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ping


https://reviews.llvm.org/D30427



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

[PATCH] D34158: For standards compatibility, preinclude if the file is available

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D34158#823316, @fedor.sergeev wrote:

> Hmm... I tried this patch and now the following worries me:
>
> - it passes -finclude-if-exists stdc-predef.h on all platforms (say, 
> including my Solaris platform that has no system stdc-predef.h)


Right, but Solaris probably _ought_ to add one as well, to define those macros.

> - it searches all the paths, not just "system include" ones
> 
>   That essentially disallows user to have stdc-predef.h include in my own 
> project, since there is a chance that this user header will be accidentally 
> included by this hidden machinery.

IMO, this is a fairly negligible issue, and so we go *shrug* oh well.

+1 for using a <> include -- that does seem better.

But, note, that will have no effect w.r.t. this issue for most users, since 
typically people use "cc -Isomepath", which adds 'somepath' to the list which 
gets searched by both <> and "" includes. Hardly anyone uses -iquote.


Repository:
  rL LLVM

https://reviews.llvm.org/D34158



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

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D34158#838454, @mibintc wrote:

> This patch is responding to @jyknight 's concern about preprocessed input. 
> The patch as it stands doesn't have this issue. I added 2 test cases, one 
> using option -x cpp-output, and another for a source file suffixed with .i
>
> Quoting James: "Firstly, let's consider a "clang foo.i" or "clang -x 
> cpp-output foo.c" compilation. In that case, it *clearly* should not be 
> including the predef file. I think the patch as it stands may not do this 
> properly. A test needs to be added for this to this patch, and perhaps the 
> behavior needs to be fixed as well."


Thanks for the test. It wasn't obvious from the code, so I'm glad to hear it 
was already correct. :)

Did you see the other suggestion I cleverly hid within a big block of 
commentary? "(TO FIX: We should be stripping the new arg as well: add 
"-fsystem-include-if-exists" argument to the list of include things in the 
skipArgs() function in lib/Driver/Job.cpp)"


https://reviews.llvm.org/D34158



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

[PATCH] D34158: For standards compatibility, preinclude if the file is available

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

This version is still disabling upon -nostdinc, which doesn't make sense to me.

You didn't remove that, nor respond explaining why you think it does make sense?


https://reviews.llvm.org/D34158



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

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Driver/ToolChains/Linux.cpp:755
+!Version.isOlderThan(4, 8, 0)) {
+  // For gcc >= 4.8.x, clang will preinclude 
+  // -ffreestanding suppresses this behavior.

jyknight wrote:
> I don't see why it makes any sense to condition this based on a GCC 
> installation existing and being >= 4.8, since this header comes from libc, 
> and is necessary for standards compliance even if there's absolutely no GCC 
> installed or involved.
> 
> Additionally, something like this -- a way for the libc to be involved in 
> setting up predefined macros -- seems probably necessary for *any* libc, if 
> they want to be strictly standards-compliant. Some of the 
> required-to-be-predefined macros like `__STDC_ISO_10646__` can only really be 
> provided by the libc, since the appropriate value is dependent on the locale 
> implementation in the libc, and not on the compiler at all.
> 
> It's possible that glibc on Linux is the only one who's cared to implement it 
> thus far (and, only when you're using GCC, at that, hence this change...). 
> That seems likely, really, since mostly the impacted macros are nearly 
> useless, so who cares... :) However, if others want to be conforming, I 
> expect they _ought_ to be using this mechanism, as well...
> 
> Thus, perhaps it ought to be an unconditional behavior of clang to include 
> the file if it exists, unless `-ffreestanding` is passed.
This is still checking GCCInstallation.isValid(), OPT_nostdinc, and only 
attempting to load the header on Linux OSes.

I don't think it shouldn't be conditioned by the former two, and I think it 
should attempt to include stdc-predef.h if it exists on *all* OSes, not just 
Linux (for the reasons described in the above comment).


https://reviews.llvm.org/D34158



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

[PATCH] D34294: Rework libcxx strerror_r handling.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL308528: Rework libcxx strerror_r handling. (authored by 
jyknight).

Repository:
  rL LLVM

https://reviews.llvm.org/D34294

Files:
  libcxx/trunk/src/system_error.cpp


Index: libcxx/trunk/src/system_error.cpp
===
--- libcxx/trunk/src/system_error.cpp
+++ libcxx/trunk/src/system_error.cpp
@@ -73,39 +73,59 @@
   std::snprintf(buffer, strerror_buff_size, "unknown error %d", ev);
   return string(buffer);
 }
-#elif defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC) && 
\
-(!defined(__ANDROID__) || __ANDROID_API__ >= 23)
-// GNU Extended version
-string do_strerror_r(int ev) {
-char buffer[strerror_buff_size];
-char* ret = ::strerror_r(ev, buffer, strerror_buff_size);
-return string(ret);
-}
 #else
-// POSIX version
+
+// Only one of the two following functions will be used, depending on
+// the return type of strerror_r:
+
+// For the GNU variant, a char* return value:
+__attribute__((unused)) const char *
+handle_strerror_r_return(char *strerror_return, char *buffer) {
+  // GNU always returns a string pointer in its return value. The
+  // string might point to either the input buffer, or a static
+  // buffer, but we don't care which.
+  return strerror_return;
+}
+
+// For the POSIX variant: an int return value.
+__attribute__((unused)) const char *
+handle_strerror_r_return(int strerror_return, char *buffer) {
+  // The POSIX variant either:
+  // - fills in the provided buffer and returns 0
+  // - returns a positive error value, or
+  // - returns -1 and fills in errno with an error value.
+  if (strerror_return == 0)
+return buffer;
+
+  // Only handle EINVAL. Other errors abort.
+  int new_errno = strerror_return == -1 ? errno : strerror_return;
+  if (new_errno == EINVAL)
+return "";
+
+  _LIBCPP_ASSERT(new_errno == ERANGE, "unexpected error from ::strerror_r");
+  // FIXME maybe? 'strerror_buff_size' is likely to exceed the
+  // maximum error size so ERANGE shouldn't be returned.
+  std::abort();
+}
+
+// This function handles both GNU and POSIX variants, dispatching to
+// one of the two above functions.
 string do_strerror_r(int ev) {
 char buffer[strerror_buff_size];
+// Preserve errno around the call. (The C++ standard requires that
+// system_error functions not modify errno).
 const int old_errno = errno;
-int ret;
-if ((ret = ::strerror_r(ev, buffer, strerror_buff_size)) != 0) {
-// If `ret == -1` then the error is specified using `errno`, otherwise
-// `ret` represents the error.
-const int new_errno = ret == -1 ? errno : ret;
-errno = old_errno;
-if (new_errno == EINVAL) {
-std::snprintf(buffer, strerror_buff_size, "Unknown error %d", ev);
-return string(buffer);
-} else {
-_LIBCPP_ASSERT(new_errno == ERANGE, "unexpected error from 
::strerr_r");
-// FIXME maybe? 'strerror_buff_size' is likely to exceed the
-// maximum error size so ERANGE shouldn't be returned.
-std::abort();
-}
+const char *error_message = handle_strerror_r_return(
+::strerror_r(ev, buffer, strerror_buff_size), buffer);
+// If we didn't get any message, print one now.
+if (!error_message[0]) {
+  std::snprintf(buffer, strerror_buff_size, "Unknown error %d", ev);
+  error_message = buffer;
 }
-return string(buffer);
+errno = old_errno;
+return string(error_message);
 }
 #endif
-
 } // end namespace
 #endif
 


Index: libcxx/trunk/src/system_error.cpp
===
--- libcxx/trunk/src/system_error.cpp
+++ libcxx/trunk/src/system_error.cpp
@@ -73,39 +73,59 @@
   std::snprintf(buffer, strerror_buff_size, "unknown error %d", ev);
   return string(buffer);
 }
-#elif defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC) && \
-(!defined(__ANDROID__) || __ANDROID_API__ >= 23)
-// GNU Extended version
-string do_strerror_r(int ev) {
-char buffer[strerror_buff_size];
-char* ret = ::strerror_r(ev, buffer, strerror_buff_size);
-return string(ret);
-}
 #else
-// POSIX version
+
+// Only one of the two following functions will be used, depending on
+// the return type of strerror_r:
+
+// For the GNU variant, a char* return value:
+__attribute__((unused)) const char *
+handle_strerror_r_return(char *strerror_return, char *buffer) {
+  // GNU always returns a string pointer in its return value. The
+  // string might point to either the input buffer, or a static
+  // buffer, but we don't care which.
+  return strerror_return;
+}
+
+// For the POSIX variant: an int return value.
+__attribute__((unused)) const char *
+handle_strerror_r_return(int strerror_return, char *buffer) {
+  // The POSIX variant either:
+  // - fills in the provided buffer and returns 0

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Driver/ToolChains/Linux.cpp:755
+!Version.isOlderThan(4, 8, 0)) {
+  // For gcc >= 4.8.x, clang will preinclude 
+  // -ffreestanding suppresses this behavior.

I don't see why it makes any sense to condition this based on a GCC 
installation existing and being >= 4.8, since this header comes from libc, and 
is necessary for standards compliance even if there's absolutely no GCC 
installed or involved.

Additionally, something like this -- a way for the libc to be involved in 
setting up predefined macros -- seems probably necessary for *any* libc, if 
they want to be strictly standards-compliant. Some of the 
required-to-be-predefined macros like `__STDC_ISO_10646__` can only really be 
provided by the libc, since the appropriate value is dependent on the locale 
implementation in the libc, and not on the compiler at all.

It's possible that glibc on Linux is the only one who's cared to implement it 
thus far (and, only when you're using GCC, at that, hence this change...). That 
seems likely, really, since mostly the impacted macros are nearly useless, so 
who cares... :) However, if others want to be conforming, I expect they _ought_ 
to be using this mechanism, as well...

Thus, perhaps it ought to be an unconditional behavior of clang to include the 
file if it exists, unless `-ffreestanding` is passed.


https://reviews.llvm.org/D34158



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

[PATCH] D31022: Implement P0298R3: `std::byte`

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ping. I don't think this got resolved, and I really wouldn't like to see  
released in this state...can you either revert this from the library, or 
implement the compiler support, before the 5.0 release branch?

In https://reviews.llvm.org/D31022#716702, @jyknight wrote:

> I believe this needs compiler support, too, in order to treat
>
>   namespace std { enum class byte : unsigned char {}; }
>
> as directly having tbaa type "omnipotent char" instead of a subtype.
>
> That is, given:
>
>   void foo(char* x, int *y) {
> x[1] = char(y[0] & 0xff);
> x[0] = char((y[0] & 0xff00) >> 8);
>   }
>
> the compiler assumes that x and y might alias each-other, and thus must have 
> two loads of y[0]. If you replace "char" with "std::byte", the same should 
> happen, but as of now does not.





https://reviews.llvm.org/D31022



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

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D34158#827178, @joerg wrote:

> I had a long discussion with James about this on IRC without reaching a clear 
> consensus. I consider forcing this behavior on all targets to be a major bug. 
> It should be opt-in and opt-in only:
>
> (1) The header name is not mandated by any standard. It is not in any 
> namespace generally accepted as implementation-owned.


This is a point. I didn't think it was a big deal, but if you want to argue a 
different name should be used, that's a reasonable argument. If we can get some 
agreement amongst other libc vendors to use some more agreeable alternative 
name, and keep a fallback on linux-only for the "stdc-predef.h" name, I'd 
consider that as a great success.

> (2) It adds magic behavior that can make debugging more difficult. Partially 
> preprocessed sources for example could be compiled with plain -c before, now 
> they need a different command line.

If this is a problem, making it be Linux-only does _nothing_ to solve it. But I 
don't actually see how this is a substantively new problem? Compiling with 
plain -c before would get #defines for those predefined macros that the 
compiler sets, even though you may not have wanted those. Is this fundamentally 
different?

> (3) It seems to me that the GNU userland (and maybe Windows) is the exception 
> to a well integrated tool chain. Most other platforms have a single canonical 
> libc, libm and libpthread implementation and can as such directly define all 
> the relevant macros directly in the driver.

I don't think this is accurate. There's many platforms out there, and for 
almost none of them do we have exact knowledge of the features of the libc 
encoded into the compiler. I'd note that not only do you need this for every 
(OS, libc) combination, you'd need it for every (OS, libc-VERSION) combination.

> Given that many of the macros involved are already reflected by the compiler 
> behavior anyway, they can't be decoupled. I.e. the questionable concept of 
> locale-independent wchar_t is already hard-coded in the front end as soon as 
> any long character literals are used.

AFAICT, this example is not actually the case. The frontend only needs to know 
*a* valid encoding for wide-character literals in some implementation-defined 
locale (for example, it might always emit them as unicode codepoints, as clang 
does).  Standard says: "the array elements [...] are initialized with the 
sequence of wide characters corresponding to the multibyte character sequence, 
as defined by the mbstowcs function with an implementation defined current 
locale."

On the other hand, I believe the intent of this macro is to guarantee that _no 
matter what_ the locale is, that a U+0100 character (say, translated with 
mbrtowc from the locale encoding) gets represented as 0x100.

Thus, it's "fine" for the frontend to always emit wchar_t literals as unicode, 
yet, the libc may sometimes use an arbitrary different internal encoding, 
depending on the locale used at runtime. (Obviously using wide character 
literals with such a libc would be a poor user experience, and such a libc 
probably ought to reconsider its choices, but that's a different discussion.)

> As such, please move the command line additions back into the target-specific 
> files for targets that actually want to get this behavior.

Without even a suggestion of a better solution to use for other targets, I find 
it is to be a real shame to push for this to be linux-only, and leave everyone 
else hanging. I find that that _most_ of these defines are effectively library 
decisions. I further would claim that these are likely to vary over the 
lifetime of even a single libc, and that as such we would be better served by 
allowing the libc to set them directly, rather than encoding it into the 
compiler.

TTBOMK, no targets except linux/glibc/gcc actually comply with this part of the 
C99/C11 standard today, and so this feature would be useful to have available 
across all targets.

(I very much dislike that the C standard has started adding all these new 
predefined macros, instead of exposing them from a header, but there's not much 
to be done about that...)

Going through the various macros:
`__STDC_ISO_10646__`:
As discussed above, this is effectively entirely up to the libc. The compiler 
only need support one possible encoding for wchar_t, and clang always chooses 
unicode. But it can't define this because the libc might use others as well.

`__STDC_MB_MIGHT_NEQ_WC__`:
As with `__STDC_ISO_10646__`, this is up to the libc not the compiler. (At 
least, I think so... I note that clang currently sets this for freebsd, with a 
FIXME next to it saying it's only intended to apply to wide character literals. 
I don't see that the standard says that, however, so, IMO, having it set for 
freebsd was and is correct).

`__STDC_UTF16__`, `__STDC_UTF32__`:
Again, analogous to `__STDC_ISO_10646__`, except dealing with 

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Just to restate: the ideal outcome of this discussion for me would be to 
resolve things such that _ALL_ libc implementations will feel comfortable using 
this technique to provide the C11-required predefined macros.

I'd love for linux, freebsd, macos, solaris, etc etc libc to all conform to the 
C standard in this regards, and do so in a common way, without the need to 
encode information about each libc version into the compiler. I _really_ don't 
think that scales well.

So I take your comments from FreeBSD's point of view seriously, and would very 
much like to understand and hopefully resolve them.

In https://reviews.llvm.org/D34158#837130, @joerg wrote:

> In https://reviews.llvm.org/D34158#836026, @jyknight wrote:
>
> > In https://reviews.llvm.org/D34158#827178, @joerg wrote:
> >
> > > (2) It adds magic behavior that can make debugging more difficult. 
> > > Partially preprocessed sources for example could be compiled with plain 
> > > -c before, now they need a different command line.
> >
> >
> > If this is a problem, making it be Linux-only does _nothing_ to solve it. 
> > But I don't actually see how this is a substantively new problem? Compiling 
> > with plain -c before
> >  would get #defines for those predefined macros that the compiler sets, 
> > even though you may not have wanted those. Is this fundamentally different?
>
>
> It makes it a linux-only problem. As such, it is something *I* only care 
> about secondary. A typical use case I care about a lot is pulling the crash 
> report sources from my (NetBSD) build machine,
>  extracting the original command line to rerun the normal compilation with 
> -save-temps. I don't necessarily have the (same) system headers on the 
> machine I use for debugging and that's exactly
>  the kind of use case this change breaks. All other predefined macros are 
> driven by the target triple and remain stable.


"it's Linux only so I don't care if it's broken." is still not very helpful. :)

But I do think understand what you're saying now, so thanks for the elaboration.

Firstly, let's consider a "clang foo.i" or "clang -x cpp-output foo.c" 
compilation. In that case, it *clearly* should not be including the predef 
file. I think the patch as it stands may not do this properly. A test needs to 
be added for this to this patch, and perhaps the behavior needs to be fixed as 
well.

(Sidenote: clang doesn't support preprocessed input properly, but that's 
another bug, and we certainly ought not make it worse. Check out e.g. "int 
main() { return __GNUC__; }". it should report that __GNUC__ is undeclared, but 
instead compiles a program that returns 4.)

But, that's not the case you're talking about above -- you're not talking about 
compiling preprocessed output, you're talking about taking output that comes 
from -frewrite-includes.

Let me recap the scenario:

1. Start with a source file foo.c, with this content:

#include 
#pragma clang __debug parser_crash

2. Run "clang foo.c". It crashes, and dumps a /tmp/foo-XXX.c and a 
/tmp/foo-XXX.sh script.

The .c file is generated via -frewrite-includes, so it's _not_ already 
preprocessed, it simply has all includes pulled into a single file. It also 
_doesn't_ insert the compiler-predefined macros at the top, but it _will_ 
include the content of this stdc-predef.h file.

OK, so then...
The generated script invokes a -cc1 command line, with all the include 
arguments stripped out of the command. (TO FIX: We should be stripping the new 
arg as well: add "-fsystem-include-if-exists" argument to the list of include 
things in the skipArgs() function in lib/Driver/Job.cpp). Even without that 
change, it's actually already fine, as there is no include path specified in 
which to find the file -- but it's cleaner to strip it, so let's do that. The 
reproducer script will thus run correctly, and not include the file.

Now, the "/tmp/foo-XXX.sh" also has a line labeled "Driver args: " with the 
original command-line on it. If I understand correctly, you then like to take 
this simpler Driver command-line, and edit it manually: add -save-temps, and 
change the input filename  to the "/tmp/foo-XXX.c" file, and run that, instead 
of actually invoking the reproducer foo-XXX.sh.

Since stdc-predef.h is included automatically, it will now be present twice -- 
first, it will read the one from your system's /usr/include, and then the copy 
inlined into the /tmp/foo-XXX.c file. That's not what you desired. You wanted 
nothing from your /usr/include to be used.

The fix for the end-user here is easy: you can add -nostdinc which will 
suppress all the default include paths, and thus it will not find this predef 
file from your system include dir.

I'll note that you'd also have had an issue if the original driver command-line 
had a "-include" option in it, which you would have needed to edit out manually 
as well. (But I understand that is less common.)

Have I correctly described the situation? I 

[PATCH] D33020: [Myriad] Pass -Xclang and -mllvm flags to moviCompile

[James Y Knight via Phabricator via cfe-commits]

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

Will submit for you since you don't have an svn account.


https://reviews.llvm.org/D33020



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

[PATCH] D29117: SPARC: allow usage of floating-point registers in inline ASM

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL302913: [SPARC] Support 'f' and 'e' inline asm constraints. 
(authored by jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D29117?vs=85708=98782#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29117

Files:
  cfe/trunk/lib/Basic/Targets.cpp
  cfe/trunk/test/CodeGen/sparcv8-inline-asm.c


Index: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -6862,6 +6862,11 @@
 case 'N': // Same as 'K' but zext (required for SIMode)
 case 'O': // The constant 4096
   return true;
+
+case 'f':
+case 'e':
+  info.setAllowsRegister();
+  return true;
 }
 return false;
   }
Index: cfe/trunk/test/CodeGen/sparcv8-inline-asm.c
===
--- cfe/trunk/test/CodeGen/sparcv8-inline-asm.c
+++ cfe/trunk/test/CodeGen/sparcv8-inline-asm.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple sparc-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s
+
+// CHECK: define float @fabsf(float %a)
+// CHECK: %{{.*}} = call float asm sideeffect "fabss $1, $0;", "=e,f"(float 
%{{.*}}) #1
+float fabsf(float a) {
+  float res;
+  __asm __volatile__("fabss  %1, %0;"
+ : /* reg out*/ "=e"(res)
+ : /* reg in */ "f"(a));
+  return res;
+}


Index: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -6862,6 +6862,11 @@
 case 'N': // Same as 'K' but zext (required for SIMode)
 case 'O': // The constant 4096
   return true;
+
+case 'f':
+case 'e':
+  info.setAllowsRegister();
+  return true;
 }
 return false;
   }
Index: cfe/trunk/test/CodeGen/sparcv8-inline-asm.c
===
--- cfe/trunk/test/CodeGen/sparcv8-inline-asm.c
+++ cfe/trunk/test/CodeGen/sparcv8-inline-asm.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple sparc-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: define float @fabsf(float %a)
+// CHECK: %{{.*}} = call float asm sideeffect "fabss $1, $0;", "=e,f"(float %{{.*}}) #1
+float fabsf(float a) {
+  float res;
+  __asm __volatile__("fabss  %1, %0;"
+ : /* reg out*/ "=e"(res)
+ : /* reg in */ "f"(a));
+  return res;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33020: [Myriad] Pass -Xclang and -mllvm flags to moviCompile

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL302738: [Myriad] Pass -Xclang and -mllvm flags to 
moviCompile (authored by jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D33020?vs=98369=98538#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33020

Files:
  cfe/trunk/lib/Driver/ToolChains/Myriad.cpp
  cfe/trunk/test/Driver/myriad-toolchain.c


Index: cfe/trunk/test/Driver/myriad-toolchain.c
===
--- cfe/trunk/test/Driver/myriad-toolchain.c
+++ cfe/trunk/test/Driver/myriad-toolchain.c
@@ -54,9 +54,11 @@
 // -fno-split-dwarf-inlining is consumed but not passed to moviCompile.
 // RUN: %clang -target shave-myriad -c -### %s -g -fno-inline-functions \
 // RUN: -fno-inline-functions-called-once -Os -Wall -MF dep.d 
-fno-split-dwarf-inlining \
-// RUN: -ffunction-sections 2>&1 | FileCheck %s -check-prefix=PASSTHRU_OPTIONS
+// RUN: -ffunction-sections -Xclang -xclangflag -mllvm -llvm-flag 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=PASSTHRU_OPTIONS
 // PASSTHRU_OPTIONS: "-g" "-fno-inline-functions" 
"-fno-inline-functions-called-once"
 // PASSTHRU_OPTIONS: "-Os" "-Wall" "-MF" "dep.d" "-ffunction-sections"
+// PASSTHRU_OPTIONS: "-Xclang" "-xclangflag" "-mllvm" "-llvm-flag"
 
 // RUN: %clang -target shave-myriad -c %s -o foo.o -### -MD -MF dep.d 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=MDMF
Index: cfe/trunk/lib/Driver/ToolChains/Myriad.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Myriad.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Myriad.cpp
@@ -43,15 +43,17 @@
   }
   CmdArgs.push_back("-DMYRIAD2");
 
-  // Append all -I, -iquote, -isystem paths, defines/undefines,
-  // 'f' flags, optimize flags, and warning options.
+  // Append all -I, -iquote, -isystem paths, defines/undefines, 'f'
+  // flags, 'g' flags, 'M' flags, optimize flags, warning options,
+  // mcpu flags, mllvm flags, and Xclang flags.
   // These are spelled the same way in clang and moviCompile.
   Args.AddAllArgsExcept(
   CmdArgs,
   {options::OPT_I_Group, options::OPT_clang_i_Group, options::OPT_std_EQ,
options::OPT_D, options::OPT_U, options::OPT_f_Group,
options::OPT_f_clang_Group, options::OPT_g_Group, options::OPT_M_Group,
-   options::OPT_O_Group, options::OPT_W_Group, options::OPT_mcpu_EQ},
+   options::OPT_O_Group, options::OPT_W_Group, options::OPT_mcpu_EQ,
+   options::OPT_mllvm, options::OPT_Xclang},
   {options::OPT_fno_split_dwarf_inlining});
   Args.hasArg(options::OPT_fno_split_dwarf_inlining); // Claim it if present.
 


Index: cfe/trunk/test/Driver/myriad-toolchain.c
===
--- cfe/trunk/test/Driver/myriad-toolchain.c
+++ cfe/trunk/test/Driver/myriad-toolchain.c
@@ -54,9 +54,11 @@
 // -fno-split-dwarf-inlining is consumed but not passed to moviCompile.
 // RUN: %clang -target shave-myriad -c -### %s -g -fno-inline-functions \
 // RUN: -fno-inline-functions-called-once -Os -Wall -MF dep.d -fno-split-dwarf-inlining \
-// RUN: -ffunction-sections 2>&1 | FileCheck %s -check-prefix=PASSTHRU_OPTIONS
+// RUN: -ffunction-sections -Xclang -xclangflag -mllvm -llvm-flag 2>&1 \
+// RUN:   | FileCheck %s -check-prefix=PASSTHRU_OPTIONS
 // PASSTHRU_OPTIONS: "-g" "-fno-inline-functions" "-fno-inline-functions-called-once"
 // PASSTHRU_OPTIONS: "-Os" "-Wall" "-MF" "dep.d" "-ffunction-sections"
+// PASSTHRU_OPTIONS: "-Xclang" "-xclangflag" "-mllvm" "-llvm-flag"
 
 // RUN: %clang -target shave-myriad -c %s -o foo.o -### -MD -MF dep.d 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=MDMF
Index: cfe/trunk/lib/Driver/ToolChains/Myriad.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Myriad.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Myriad.cpp
@@ -43,15 +43,17 @@
   }
   CmdArgs.push_back("-DMYRIAD2");
 
-  // Append all -I, -iquote, -isystem paths, defines/undefines,
-  // 'f' flags, optimize flags, and warning options.
+  // Append all -I, -iquote, -isystem paths, defines/undefines, 'f'
+  // flags, 'g' flags, 'M' flags, optimize flags, warning options,
+  // mcpu flags, mllvm flags, and Xclang flags.
   // These are spelled the same way in clang and moviCompile.
   Args.AddAllArgsExcept(
   CmdArgs,
   {options::OPT_I_Group, options::OPT_clang_i_Group, options::OPT_std_EQ,
options::OPT_D, options::OPT_U, options::OPT_f_Group,
options::OPT_f_clang_Group, options::OPT_g_Group, options::OPT_M_Group,
-   options::OPT_O_Group, options::OPT_W_Group, options::OPT_mcpu_EQ},
+   options::OPT_O_Group, options::OPT_W_Group, options::OPT_mcpu_EQ,
+   options::OPT_mllvm, options::OPT_Xclang},
   {options::OPT_fno_split_dwarf_inlining});
   Args.hasArg(options::OPT_fno_split_dwarf_inlining); // Claim it if present.
 
___
cfe-commits 

[PATCH] D34294: Rework libcxx strerror_r handling.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

EricWF, wdyt?


https://reviews.llvm.org/D34294



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

[PATCH] D34294: Rework libcxx strerror_r handling.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
Herald added subscribers: krytarowski, srhines.

The set of #ifdefs used to handle the two incompatible variants of
strerror_r were not complete (they didn't handle newlib appropriately).

Rather than attempting to make the ifdefs more complex, make them
unnecessary by choosing which behavior to use dependent upon the
return type.


https://reviews.llvm.org/D34294

Files:
  src/system_error.cpp


Index: src/system_error.cpp
===
--- src/system_error.cpp
+++ src/system_error.cpp
@@ -73,39 +73,59 @@
   std::snprintf(buffer, strerror_buff_size, "unknown error %d", ev);
   return string(buffer);
 }
-#elif defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC) && 
\
-(!defined(__ANDROID__) || __ANDROID_API__ >= 23)
-// GNU Extended version
-string do_strerror_r(int ev) {
-char buffer[strerror_buff_size];
-char* ret = ::strerror_r(ev, buffer, strerror_buff_size);
-return string(ret);
-}
 #else
-// POSIX version
+
+// Only one of the two following functions will be used, depending on
+// the return type of strerror_r:
+
+// For the GNU variant, a char* return value:
+__attribute__((unused)) const char *
+handle_strerror_r_return(char *strerror_return, char *buffer) {
+  // GNU always returns a string pointer in its return value. The
+  // string might point to either the input buffer, or a static
+  // buffer, but we don't care which.
+  return strerror_return;
+}
+
+// For the POSIX variant: an int return value.
+__attribute__((unused)) const char *
+handle_strerror_r_return(int strerror_return, char *buffer) {
+  // The POSIX variant either:
+  // - fills in the provided buffer and returns 0
+  // - returns a positive error value, or
+  // - returns -1 and fills in errno with an error value.
+  if (strerror_return == 0)
+return buffer;
+
+  // Only handle EINVAL. Other errors abort.
+  int new_errno = strerror_return == -1 ? errno : strerror_return;
+  if (new_errno == EINVAL)
+return "";
+
+  _LIBCPP_ASSERT(new_errno == ERANGE, "unexpected error from ::strerror_r");
+  // FIXME maybe? 'strerror_buff_size' is likely to exceed the
+  // maximum error size so ERANGE shouldn't be returned.
+  std::abort();
+}
+
+// This function handles both GNU and POSIX variants, dispatching to
+// one of the two above functions.
 string do_strerror_r(int ev) {
 char buffer[strerror_buff_size];
+// Preserve errno around the call. (The C++ standard requires that
+// system_error functions not modify errno).
 const int old_errno = errno;
-int ret;
-if ((ret = ::strerror_r(ev, buffer, strerror_buff_size)) != 0) {
-// If `ret == -1` then the error is specified using `errno`, otherwise
-// `ret` represents the error.
-const int new_errno = ret == -1 ? errno : ret;
-errno = old_errno;
-if (new_errno == EINVAL) {
-std::snprintf(buffer, strerror_buff_size, "Unknown error %d", ev);
-return string(buffer);
-} else {
-_LIBCPP_ASSERT(new_errno == ERANGE, "unexpected error from 
::strerr_r");
-// FIXME maybe? 'strerror_buff_size' is likely to exceed the
-// maximum error size so ERANGE shouldn't be returned.
-std::abort();
-}
+const char *error_message = handle_strerror_r_return(
+::strerror_r(ev, buffer, strerror_buff_size), buffer);
+// If we didn't get any message, print one now.
+if (!error_message[0]) {
+  std::snprintf(buffer, strerror_buff_size, "Unknown error %d", ev);
+  error_message = buffer;
 }
-return string(buffer);
+errno = old_errno;
+return string(error_message);
 }
 #endif
-
 } // end namespace
 #endif
 


Index: src/system_error.cpp
===
--- src/system_error.cpp
+++ src/system_error.cpp
@@ -73,39 +73,59 @@
   std::snprintf(buffer, strerror_buff_size, "unknown error %d", ev);
   return string(buffer);
 }
-#elif defined(__linux__) && !defined(_LIBCPP_HAS_MUSL_LIBC) && \
-(!defined(__ANDROID__) || __ANDROID_API__ >= 23)
-// GNU Extended version
-string do_strerror_r(int ev) {
-char buffer[strerror_buff_size];
-char* ret = ::strerror_r(ev, buffer, strerror_buff_size);
-return string(ret);
-}
 #else
-// POSIX version
+
+// Only one of the two following functions will be used, depending on
+// the return type of strerror_r:
+
+// For the GNU variant, a char* return value:
+__attribute__((unused)) const char *
+handle_strerror_r_return(char *strerror_return, char *buffer) {
+  // GNU always returns a string pointer in its return value. The
+  // string might point to either the input buffer, or a static
+  // buffer, but we don't care which.
+  return strerror_return;
+}
+
+// For the POSIX variant: an int return value.
+__attribute__((unused)) const char *
+handle_strerror_r_return(int strerror_return, char 

[PATCH] D34294: Rework libcxx strerror_r handling.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D34294#782806, @krytarowski wrote:

> New one is harder to comprehend and less portable (usage of `__atribute__`).


I can't disagree more strongly.  This is fundamentally portable C++ code -- 
__attribute__((unused)) is simply warning suppression.
I used it directly, because I saw other .cpp files in libcxx already did so. If 
it needs to be conditioned, it could be.

> Or better:



> #elif __GLIBC__ || (__ANDROID_API__ - 0) >= 23
>  If I understand correctly, Android adopted GNU version.

No, we can't use that conditional, because android and glibc aren't the only 
ones to have adopted the GNU variant. As I mentioned, newlib uses it also.

We *may* be able to use a different conditional than what's there or what you 
suggested, but solving the problem once and for all by using the function type 
is certainly a better solution.


https://reviews.llvm.org/D34294



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

[PATCH] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305394: PR32476: __nop_locale_mgmt.h not needed with newlib 
2.5+ (authored by jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D32146?vs=102265=102562#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D32146

Files:
  libcxx/trunk/include/support/newlib/xlocale.h


Index: libcxx/trunk/include/support/newlib/xlocale.h
===
--- libcxx/trunk/include/support/newlib/xlocale.h
+++ libcxx/trunk/include/support/newlib/xlocale.h
@@ -16,7 +16,10 @@
 #include 
 #include 
 #include 
+#if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \
+__NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5
 #include 
+#endif
 #include 
 #include 
 


Index: libcxx/trunk/include/support/newlib/xlocale.h
===
--- libcxx/trunk/include/support/newlib/xlocale.h
+++ libcxx/trunk/include/support/newlib/xlocale.h
@@ -16,7 +16,10 @@
 #include 
 #include 
 #include 
+#if !defined(__NEWLIB__) || __NEWLIB__ < 2 || \
+__NEWLIB__ == 2 && __NEWLIB_MINOR__ < 5
 #include 
+#endif
 #include 
 #include 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34105: Define _GNU_SOURCE for rtems c++

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305399: Define _GNU_SOURCE for rtems c++ (authored by 
jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D34105?vs=102188=102565#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34105

Files:
  cfe/trunk/lib/Basic/Targets.cpp
  cfe/trunk/test/Preprocessor/init.c


Index: cfe/trunk/test/Preprocessor/init.c
===
--- cfe/trunk/test/Preprocessor/init.c
+++ cfe/trunk/test/Preprocessor/init.c
@@ -8779,6 +8779,7 @@
 // KFREEBSDI686-DEFINE:#define __GLIBC__ 1
 //
 // RUN: %clang_cc1 -x c++ -triple i686-pc-linux-gnu -fobjc-runtime=gcc -E -dM 
< /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s
+// RUN: %clang_cc1 -x c++ -triple sparc-rtems-elf -E -dM < /dev/null | 
FileCheck -match-full-lines -check-prefix GNUSOURCE %s
 // GNUSOURCE:#define _GNU_SOURCE 1
 //
 // RUN: %clang_cc1 -x c++ -std=c++98 -fno-rtti -E -dM < /dev/null | FileCheck 
-match-full-lines -check-prefix NORTTI %s
Index: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -4734,6 +4734,9 @@
 
 Builder.defineMacro("__rtems__");
 Builder.defineMacro("__ELF__");
+// Required by the libc++ locale support.
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
   }
 
 public:


Index: cfe/trunk/test/Preprocessor/init.c
===
--- cfe/trunk/test/Preprocessor/init.c
+++ cfe/trunk/test/Preprocessor/init.c
@@ -8779,6 +8779,7 @@
 // KFREEBSDI686-DEFINE:#define __GLIBC__ 1
 //
 // RUN: %clang_cc1 -x c++ -triple i686-pc-linux-gnu -fobjc-runtime=gcc -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s
+// RUN: %clang_cc1 -x c++ -triple sparc-rtems-elf -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix GNUSOURCE %s
 // GNUSOURCE:#define _GNU_SOURCE 1
 //
 // RUN: %clang_cc1 -x c++ -std=c++98 -fno-rtti -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix NORTTI %s
Index: cfe/trunk/lib/Basic/Targets.cpp
===
--- cfe/trunk/lib/Basic/Targets.cpp
+++ cfe/trunk/lib/Basic/Targets.cpp
@@ -4734,6 +4734,9 @@
 
 Builder.defineMacro("__rtems__");
 Builder.defineMacro("__ELF__");
+// Required by the libc++ locale support.
+if (Opts.CPlusPlus)
+  Builder.defineMacro("_GNU_SOURCE");
   }
 
 public:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30427: Fix whitespace before token-paste of an argument.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL302195: Fix whitespace before token-paste of an argument. 
(authored by jyknight).

Changed prior to commit:
  https://reviews.llvm.org/D30427?vs=89918=97881#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30427

Files:
  cfe/trunk/lib/Lex/TokenLexer.cpp
  cfe/trunk/test/Preprocessor/macro_paste_commaext.c
  cfe/trunk/test/Preprocessor/stringize_space.c

Index: cfe/trunk/test/Preprocessor/macro_paste_commaext.c
===
--- cfe/trunk/test/Preprocessor/macro_paste_commaext.c
+++ cfe/trunk/test/Preprocessor/macro_paste_commaext.c
@@ -1,13 +1,20 @@
-// RUN: %clang_cc1 %s -E | grep 'V);'
-// RUN: %clang_cc1 %s -E | grep 'W, 1, 2);'
-// RUN: %clang_cc1 %s -E | grep 'X, 1, 2);'
-// RUN: %clang_cc1 %s -E | grep 'Y,);'
-// RUN: %clang_cc1 %s -E | grep 'Z,);'
+// RUN: %clang_cc1 %s -E | FileCheck --strict-whitespace --match-full-lines %s
+
+// In the following tests, note that the output is sensitive to the
+// whitespace *preceeding* the varargs argument, as well as to
+// interior whitespace. AFAIK, this is the only case where whitespace
+// preceeding an argument matters, and might be considered a bug in
+// GCC. Nevertheless, since this feature is a GCC extension in the
+// first place, we'll follow along.
 
 #define debug(format, ...) format, ## __VA_ARGS__)
+// CHECK:V);
 debug(V);
-debug(W, 1, 2);
+// CHECK:W,1, 2);
+debug(W,1, 2);
+// CHECK:X, 1, 2);
 debug(X, 1, 2 );
+// CHECK:Y,);
 debug(Y, );
+// CHECK:Z,);
 debug(Z,);
-
Index: cfe/trunk/test/Preprocessor/stringize_space.c
===
--- cfe/trunk/test/Preprocessor/stringize_space.c
+++ cfe/trunk/test/Preprocessor/stringize_space.c
@@ -18,3 +18,14 @@
 1)
 
 // CHECK: {{^}}"-1"
+
+#define paste(a,b) str(a ".foo" not ". foo".
 if (i != 0 && !Tokens[i-1].is(tok::hashhash) && CurTok.hasLeadingSpace())
   NextTokGetsSpace = true;
 
@@ -317,14 +323,16 @@
 const Token *ArgToks = ActualArgs->getUnexpArgument(ArgNo);
 unsigned NumToks = MacroArgs::getArgLength(ArgToks);
 if (NumToks) {  // Not an empty argument?
+  bool VaArgsPseudoPaste = false;
   // If this is the GNU ", ## __VA_ARGS__" extension, and we just learned
   // that __VA_ARGS__ expands to multiple tokens, avoid a pasting error when
   // the expander trys to paste ',' with the first token of the __VA_ARGS__
   // expansion.
   if (NonEmptyPasteBefore && ResultToks.size() >= 2 &&
   ResultToks[ResultToks.size()-2].is(tok::comma) &&
   (unsigned)ArgNo == Macro->getNumArgs()-1 &&
   Macro->isVariadic()) {
+VaArgsPseudoPaste = true;
 // Remove the paste operator, report use of the extension.
 PP.Diag(ResultToks.pop_back_val().getLocation(), diag::ext_paste_comma);
   }
@@ -344,18 +352,16 @@
ResultToks.end()-NumToks, ResultToks.end());
   }
 
-  // If this token (the macro argument) was supposed to get leading
-  // whitespace, transfer this information onto the first token of the
-  // expansion.
-  //
-  // Do not do this if the paste operator occurs before the macro argument,
-  // as in "A ## MACROARG".  In valid code, the first token will get
-  // smooshed onto the preceding one anyway (forming AMACROARG).  In
-  // assembler-with-cpp mode, invalid pastes are allowed through: in this
-  // case, we do not want the extra whitespace to be added.  For example,
-  // we want ". ## foo" -> ".foo" not ". foo".
-  if (NextTokGetsSpace)
-ResultToks[ResultToks.size()-NumToks].setFlag(Token::LeadingSpace);
+  // Transfer the leading whitespace information from the token
+  // (the macro argument) onto the first token of the
+  // expansion. Note that we don't do this for the GNU
+  // pseudo-paste extension ", ## __VA_ARGS__".
+  if (!VaArgsPseudoPaste) {
+ResultToks[ResultToks.size() - 

[PATCH] D35755: [Solaris] gcc toolchain handling revamp

[James Y Knight via Phabricator via cfe-commits]

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

This looks reasonable on the face of it.

I'm assuming you know the layout for Solaris, and it doesn't seem to change the 
behavior of non-Solaris, so LGTM.


https://reviews.llvm.org/D35755



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

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I'd still _very_ much like a solution that is acceptable for all libc to use, 
and have that on by default.

However, I guess this is fine.

Only concern I have is that it seems odd that so many test-cases needed to be 
changed. What fails without those test changes?


https://reviews.llvm.org/D34158



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

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I think the diagnosis on the original issue was incorrect.

It seems to me that it was caused by the prefix being set as "/bin" instead of 
"/usr/bin", because clang _doesn't_ actually canonicalize its prefix, even when 
-no-canonical-prefixes isn't specified! All it does, now, is to make the prefix 
absolute -- without fully canonicalizing. I think that's simply a bug.

If the prefix had been, properly, /usr/bin, then the include path would've been:

  
/usr/bin/../lib64/gcc/x86_64-pc-linux-gnu/7.2.0/../../../../include/c++/7.2.0/iostream

And in that case, it would've worked properly with ninja, without adding the 
feature in this patch.

Reference clang/tools/driver/driver.cpp:297:

  // FIXME: We don't actually canonicalize this, we just make it absolute.
  if (CanonicalPrefixes)
llvm::sys::fs::make_absolute(InstalledPath);


Repository:
  rL LLVM

https://reviews.llvm.org/D37954



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

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D42933#1090384, @jfb wrote:

> In https://reviews.llvm.org/D42933#1090286, @smeenai wrote:
>
> > I'd be fine with adding an option to relax the printf checking if the size 
> > and alignment of the specifier and the actual type match, even if the types 
> > themselves differ (`-Wformat-relaxed` or something similar), so that you'd 
> > still get warnings on cases where the specifier mismatch could cause 
> > runtime issues.
>
>
> What are the cases that you're worried about? The only ones I'm trying to 
> capture here are `NSInteger` with `%zd` and `NSUInteger` with `%zu`, are 
> there others?
>
> > I think that would be preferable to special-casing the Apple types.
>
> If there are more that should be captured and a similar point solution 
> doesn't apply, agreed. However I'd like to understand if we agree on the 
> guarantees that the platform offers for the two specific cases I'm targeting.


I also think that special casing these two specifiers doesn't make sense. The 
problem is a general issue -- and one I've often found irritating. This exact 
same situation comes up all the time in non-Darwin contexts too.

E.g. one I find particularly annoying is "%lld" vs "%ld" in printf.

Some 64-bit platforms define e.g. `int64_t` as `long long`, and others as 
`long`. Although both types are size 8, align 8, and mean exactly the same 
thing, they are still distinct. And so, users write code like `int64_t x = 0; 
printf("%ld", x);`...which then emits warnings on the platform that defines 
int64_t as a `long long`. Obviously, the code _could_ be using `PRId64` 
instead...but it often doesn't. So it'd sure be nice if you could restrict the 
warning to only warn about actual problems, and suppress these 
superficially-incompatible-but-not-really warnings.

So, +1 from me for the general-purpose `-Wformat-relaxed` flag (or whatever 
other flag name is appropriate).


Repository:
  rC Clang

https://reviews.llvm.org/D42933



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

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D42933#1091817, @jfb wrote:

> In https://reviews.llvm.org/D42933#1091809, @jyknight wrote:
>
> > I also think that special casing these two specifiers doesn't make sense. 
> > The problem is a general issue -- and one I've often found irritating. This 
> > exact same situation comes up all the time in non-Darwin contexts too.
>
>
> I don't think that's true. In this very specific case the platform guarantees 
> that `(sizeof(size_t) == sizeof(NSInteger)) && (sizeof(ssize_t) == 
> sizeof(NSUInteger))` for all architectures this platform supports. This exact 
> same situation does not come up all the time in other contexts because the 
> `int` / `long` / `long long` distinction isn't backed by a portability 
> guarantee. The warning is there to say "this code isn't portable!", but in 
> the very specific case of `NSInteger` and `NSUInteger` it is and users rely 
> on it so it cannot be broken. The warning is therefore spurious, users 
> therefore rightly complain.


The printf format specifier warning is not primarily a cross-platform 
portability checker. And, although in a few limited cases it can act somewhat 
like one, in general it does not. Take my previous example -- you get no 
warning on a platform that has int64_t as a typedef for long -- if this feature 
is to be useful as a portability checker, it should require that you used the 
PRId64 macro. Or, given `ssize_t x = 0; printf("%ld", x);`, it doesn't tell you 
to use "%zd" instead if ssize_t is a typedef for long -- although to be 
portable you ought to.

No, the major usefulness of the printf warning is to tell you that your code is 
incorrect for the _current_ platform. And //most// importantly when you chose 
the wrong size for your argument.

Those types which have matched size and alignment are still different types, 
and so it's technically appropriate to warn about using the wrong specifier 
there, too. But it's also entirely reasonable to not want to be bothered by 
such useless trivia, so skipping the warning when there's only a technical and 
not actual mismatch seems entirely sensible. (I might suggest that it should 
even be the default behavior for the warning, and if you want the stricter 
checks you'd ask for them...but I bet I'll get more pushback on that...)


Repository:
  rC Clang

https://reviews.llvm.org/D42933



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

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

There are more leon-based v8 CPUs listed in clang/lib/Basic/Targets/Sparc.cpp, 
which need to be listed here, also.

Separately, I think it'd be a good idea to refactor to put this info into the 
SparcCPUInfo struct, so that it's harder for them to get out of sync.


Repository:
  rC Clang

https://reviews.llvm.org/D47138



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

[PATCH] D47137: [Sparc] Add floating-point register names

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Can you add a test that we support this? 
clang/test/CodeGen/sparcv8-inline-asm.c would be a good place to add a test 
case similar to that in clang/test/CodeGen/aarch64-inline-asm.c : 
test_gcc_registers.


Repository:
  rC Clang

https://reviews.llvm.org/D47137



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

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

[James Y Knight via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D47138#1107637, @dcederman wrote:

> I did not find a good way to access the SparcCPUInfo struct from here. No 
> other arch under Toolchains seems to access TargetInfo.


OK.


https://reviews.llvm.org/D47138



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

[PATCH] D47137: [Sparc] Add floating-point register names

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Basic/Targets/Sparc.cpp:32
+"f22", "f23", "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", 
"f32",
+"f33", "f34", "f35", "f36", "f37", "f38", "f39", "f40", "f41", "f42", 
"f43",
+"f44", "f45", "f46", "f47", "f48", "f49", "f50", "f51", "f52", "f53", 
"f54",

There's no such register as f33 (nor any odd-numbered reg above f32).



Comment at: lib/Basic/Targets/Sparc.cpp:52
+
+// Double precision floating-point register
+{{"d0"}, "f0"},   {{"d1"}, "f2"},   {{"d2"}, "f4"},   {{"d3"}, "f6"},

AFAICT, gcc doesn't actually accept "d" and "q" aliases in its inline asm, so 
probably no point in LLVM doing so either.


https://reviews.llvm.org/D47137



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

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D47894#1125961, @srhines wrote:

> In https://reviews.llvm.org/D47894#1125728, @jyknight wrote:
>
> > In https://reviews.llvm.org/D47894#1125653, @efriedma wrote:
> >
> > > The problem would come from propagating nonnull-ness from something which 
> > > isn't inherently nonnull.  For example, strlen has a nonnull argument, so 
> > > `strlen(NULL)` is UB, therefore given `int z = strlen(x); if (x) {...}`, 
> > > we can remove the null check.  (Not sure we actually do this transform at 
> > > the moment, but it's something we could do in the future.)
> >
> >
> > I think the question there is actually whether we need to, in addition to 
> > supporting null pointer dereference, also cause all 
> > `__attribute__((nonnull))` annotations at the C/C++ level to be ignored.
> >
> > And, yes, I believe we should.
>
>
> Is that what the kernel community actually wants though? There are certainly 
> others who want a way to strip `__attribute__((nonnull))` completely, which 
> seems a bit orthogonal to the removal of null checks in general. I think it 
> would be good to support such a flag, but the presence of `nonnull` inside 
> kernel sources leads me to believe they want those cases to be treated 
> specially.


In GCC, the nonnull attribute still has an effect on warnings but not on 
codegen if you enable -fno-delete-null-pointer-checks: 
https://godbolt.org/g/7SSi2V

That seems a pretty sensible behavior, IMO.


Repository:
  rC Clang

https://reviews.llvm.org/D47894



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

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D47894#1125653, @efriedma wrote:

> The problem would come from propagating nonnull-ness from something which 
> isn't inherently nonnull.  For example, strlen has a nonnull argument, so 
> `strlen(NULL)` is UB, therefore given `int z = strlen(x); if (x) {...}`, we 
> can remove the null check.  (Not sure we actually do this transform at the 
> moment, but it's something we could do in the future.)


I think the question there is actually whether we need to, in addition to 
supporting null pointer dereference, also cause all __attribute__((nonnull)) 
annotations at the C/C++ level to be ignored.

And, yes, I believe we should.


Repository:
  rC Clang

https://reviews.llvm.org/D47894



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

[PATCH] D42742: [clangd] Use pthread instead of thread_local to support more runtimes.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Is there some way to figure out what's going on in 
clang-x86_64-linux-selfhost-modules?

I believe there should be no linux builders which are missing this function -- 
it was added to libgcc in 4.8, and we don't support older versions, so I think 
missing this function on a linux build must be a misconfiguration of some sort.


Repository:
  rL LLVM

https://reviews.llvm.org/D42742



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

[PATCH] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

[James Y Knight via Phabricator via cfe-commits]

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

Typo in commit message? They were added to 2.5, not 2.4 (the code is right, 
just the comment is wrong).


Repository:
  rCXX libc++

https://reviews.llvm.org/D49927



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

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

[James Y Knight via Phabricator via cfe-commits]

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

Please refer to the commit for the LLVM half of the change in the commit 
message for this.

LGTM, other than some minor suggestions for the help texts.




Comment at: docs/ClangCommandLineReference.rst:1548
+
+Assume that null pointers cannot be dereferenced safely and any code/data 
cannot reside at address zero.
+

Suggested text:
When enabled, treat null pointer dereference, creation of a reference to null, 
or passing a null pointer to a function parameter annotated with the "nonnull" 
attribute as undefined behavior.  (And, thus the optimizer may assume that any 
pointer used in such a way must not have been null and optimize away branches 
accordingly.) On by default.




Comment at: include/clang/Driver/Options.td:1081
+  "fdelete-null-pointer-checks">, Group,
+  HelpText<"Assume that null pointers can not be dereferenced safely, and 
remove useless null checks">;
+def fno_delete_null_pointer_checks : Flag<["-"],

Suggested text:
Treat usage of null pointers as undefined behavior.



Comment at: include/clang/Driver/Options.td:1084
+  "fno-delete-null-pointer-checks">, Group, Flags<[CC1Option]>,
+  HelpText<"Assume that it may be possible to safely dereference null 
pointers">;
+

Suggested text:
Do not treat usage of null pointers as undefined behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D47894



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

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In https://reviews.llvm.org/D47894#1158811, @manojgupta wrote:

> @efriedma @jyknight Does the change match your expectations where warnings 
> are still generated but codeGen does not emit nonnull attribute?


Yes, this seems sensible IMO.




Comment at: include/clang/Driver/Options.td:1080
+def fdelete_null_pointer_checks : Flag<["-"],
+  "fdelete-null-pointer-checks">, Group;
+def fno_delete_null_pointer_checks : Flag<["-"],

Do we need help text for this one too?



Comment at: test/CodeGen/nonnull.c:1-2
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm < %s | FileCheck 
-check-prefix=PREFIX-NONNULL %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm 
-fno-delete-null-pointer-checks < %s | FileCheck -check-prefix=PREFIX-NO-NULL %s
 

These prefixes are very confusingly named. NONNULL and NO-NULL sound like the 
same thing.
I'd suggest using, across all the files here,
CHECK:
NULL-INVALID:
NULL-VALID:

That'd also avoid the need to rename all the "CHECK" lines to something else, 
which is most of the diff in these files.



Comment at: test/Sema/nonnull.c:2
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fno-delete-null-pointer-checks -verify %s
 // rdar://9584012

manojgupta wrote:
> All warnings are still issued if nullptr is passed to function nonnull 
> attribute.
SGTM, but this should be a comment in the file, not the code review.


Repository:
  rC Clang

https://reviews.llvm.org/D47894



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

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

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

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

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


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


https://reviews.llvm.org/D52918



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

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

[James Y Knight via Phabricator via cfe-commits]

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

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

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

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

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

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


https://reviews.llvm.org/D52939

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

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

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

[James Y Knight via Phabricator via cfe-commits]

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

Added test.


https://reviews.llvm.org/D52918

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


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


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

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

[James Y Knight via Phabricator via cfe-commits]

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

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

Repository:
  rC Clang

https://reviews.llvm.org/D52919

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


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


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

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

[James Y Knight via Phabricator via cfe-commits]

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

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

Repository:
  rL LLVM

https://reviews.llvm.org/D52918

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


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


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

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

[James Y Knight via Phabricator via cfe-commits]

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

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

Repository:
  rL LLVM

https://reviews.llvm.org/D52918

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


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


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

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

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

It is intended to disable _all_ warnings, even those upgraded to
errors via `-Werror=warningname` or `#pragma clang diagnostic error'


https://reviews.llvm.org/D53199

Files:
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Frontend/warning-mapping-4.c
  clang/test/Frontend/warning-mapping-5.c
  clang/test/Frontend/warning-mapping-6.c


Index: clang/test/Frontend/warning-mapping-6.c
===
--- /dev/null
+++ clang/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: clang/test/Frontend/warning-mapping-5.c
===
--- clang/test/Frontend/warning-mapping-5.c
+++ clang/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: clang/test/Frontend/warning-mapping-4.c
===
--- clang/test/Frontend/warning-mapping-4.c
+++ clang/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 // RUN: %clang_cc1 -verify -Wno-error=sign-compare %s
 // RUN: %clang_cc1 -verify -Wsign-compare -w -Wno-error=sign-compare %s
+// RUN: %clang_cc1 -verify -w -Werror=sign-compare %s
 // expected-no-diagnostics
 
 int f0(int x, unsigned y) {
Index: clang/test/Frontend/warning-mapping-2.c
===
--- clang/test/Frontend/warning-mapping-2.c
+++ clang/test/Frontend/warning-mapping-2.c
@@ -1,5 +1,7 @@
-// Check that -w has lower priority than -pedantic-errors.
+// Check that -w takes precedence over -pedantic-errors.
 // RUN: %clang_cc1 -verify -pedantic-errors -w %s
 
-void f0() { f1(); } // expected-error {{implicit declaration of function}}
+// Expect *not* to see a diagnostic for "implicit declaration of function"
+// expected-no-diagnostics
 
+void f0() { f1(); }
Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -457,12 +457,15 @@
   if (Result == diag::Severity::Ignored)
 return Result;
 
-  // Honor -w, which is lower in priority than pedantic-errors, but higher than
-  // -Werror.
-  // FIXME: Under GCC, this also suppresses warnings that have been mapped to
-  // errors by -W flags and #pragma diagnostic.
-  if (Result == diag::Severity::Warning && State->IgnoreAllWarnings)
-return diag::Severity::Ignored;
+  // Honor -w: this disables all messages mapped to Warning severity, and 
*also*
+  // any diagnostics which are not Error/Fatal by default (that is, they were
+  // upgraded by any of the mechanisms available: -Werror, -pedantic, or 
#pragma
+  // diagnostic)
+  if (State->IgnoreAllWarnings) {
+if (Result == diag::Severity::Warning ||
+!isDefaultMappingAsError((diag::kind)DiagID))
+  return diag::Severity::Ignored;
+  }
 
   // If -Werror is enabled, map warnings to errors unless explicitly disabled.
   if (Result == diag::Severity::Warning) {


Index: clang/test/Frontend/warning-mapping-6.c
===
--- /dev/null
+++ clang/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: clang/test/Frontend/warning-mapping-5.c
===
--- clang/test/Frontend/warning-mapping-5.c
+++ clang/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: clang/test/Frontend/warning-mapping-4.c
===
--- clang/test/Frontend/warning-mapping-4.c
+++ clang/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 

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

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 169641.
jyknight marked 11 inline comments as done.
jyknight added a comment.

Address most comments.


https://reviews.llvm.org/D52939

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

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

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

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:12-32
 // Constant expression evaluation produces four main results:
 //
 //  * A success/failure flag indicating whether constant folding was 
successful.
 //This is the 'bool' return value used by most of the code in this file. A
 //'false' return value indicates that constant folding has failed, and any
 //appropriate diagnostic has already been produced.
 //

rsmith wrote:
> Is this comment still correct?
Yes -- the "potential" constant expression mode still has this semantic.



Comment at: clang/lib/AST/ExprConstant.cpp:682
 enum EvaluationMode {
-  /// Evaluate as a constant expression. Stop if we find that the 
expression
-  /// is not a constant expression.
+  /* The various EvaluationMode kinds vary in terms of what sorts of
+   * expressions they accept.

rsmith wrote:
> Nit: LLVM style avoids `/*...*/` comments even for long comment blocks like 
> this (https://llvm.org/docs/CodingStandards.html#comment-formatting).
Done.



Comment at: clang/lib/AST/ExprConstant.cpp:686-687
+ Key for the following table:
+ * D = Diagnostic notes (about non-constexpr, but still evaluable
+   constructs) are OK (keepEvaluatingAfterNote)
+ * S = Failure to evaluate which only occurs in expressions used for

rsmith wrote:
> I think talking about diagnostic notes here distracts from the purpose, which 
> is that this column indicates that we accept constructs that are not 
> permitted by the language mode's constant expression rules.
Reworded.



Comment at: clang/lib/AST/ExprConstant.cpp:688-690
+ * S = Failure to evaluate which only occurs in expressions used for
+   side-effect are okay.  (keepEvaluatingAfterSideEffect)
+ * F = Failure to evaluate is okay. (keepEvaluatingAfterFailure)

rsmith wrote:
> "is okay" here is misleading, I think. The point of 
> `keepEvaluatingAfter[...]` is that it's safe to stop evaluating if they 
> return false, because later evaluations can't affect the overall result.
The overall return value of the evaluation indicates whether the expression was 
evaluable under a specified set of conditions. The different modes cause 
failure under different conditions -- so I think "is okay" really _is_ what is 
meant here. In this particular case, "failure to evaluate" can still return 
success!

Not sure if some rewording would communicate that better?



Comment at: clang/lib/AST/ExprConstant.cpp:692
+ * E = Eagerly evaluate certain builtins to a value which would 
normally
+   defer until after optimizations.
+

rsmith wrote:
> defer -> be deferred?
Done.



Comment at: clang/lib/AST/ExprConstant.cpp:706-707
+
+ TODO: Fix EM_ConstantExpression and EM_PotentialConstantExpression to
+ also eagerly evaluate. (and then delete
+ EM_PotentialConstantExpressionUnevaluated as a duplicate)

rsmith wrote:
> Allowing `EM_ConstantExpression` to evaluate expressions that 
> `EM_ConstantFold` cannot evaluate would violate our invariants. We rely on 
> being able to check up front that an expression is a constant expression and 
> then later just ask for its value, and for the later query to always succeed 
> and return the same value as the earlier one. (This is one of the reasons why 
> I suggested in D52854 that we add an explicit AST representation for constant 
> expressions.)
I agree, it would right now. But I do think it should be fixed not to in the 
future. Let me reword the TODO, noting that it's not trivial.



Comment at: clang/lib/AST/ExprConstant.cpp:711
+
+  /// Evaluate as a constant expression, as per C++11-and-later constexpr
+  /// rules. Stop if we find that the expression is not a constant

rsmith wrote:
> This is not the intent. The evaluator uses the rules of the current language 
> mode. However, it doesn't enforce the C and C++98 syntactic restrictions on 
> what can appear in a constant expression, because it turns out to be cleaner 
> to check those separately rather than to interleave them with evaluation.
> 
> We should probably document this as evaluating following the evaluation (but 
> not syntactic) constant expression rules of the current language mode.
I'm not really sure what you mean to distinguish between C/C++98 "evaluation 
constant expression rules" and the definition of a C/C++98 ICE?

Perhaps you mean that if evaluation succeeds, it will have produced a value 
consistent with the rules of the current language, but that it may not produce 
the diagnostics in all cases it ought to?

AFAICT, the evaluator is designed to emit diagnostics only for the C++11 (and 
later) constant expression rules, as well as for inability to 

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

[James Y Knight via Phabricator via cfe-commits]

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

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

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


https://reviews.llvm.org/D52919

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


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


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

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

[James Y Knight via Phabricator via cfe-commits]

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

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

This is an alternate solution to r336746.


https://reviews.llvm.org/D52918

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaExpr.cpp


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5846,7 +5846,9 @@
   // pointers.  Everything else should be possible.
 
   QualType SrcTy = Src.get()->getType();
-  if (Context.hasSameUnqualifiedType(SrcTy, DestTy))
+  // We can use a no-op cast if the types are the same, or only adding/removing
+  // const, volatile, or restricted qualifiers.
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
 return CK_NoOp;
 
   switch (Type::ScalarTypeKind SrcKind = SrcTy->getScalarTypeKind()) {
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5861,11 +5861,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), 
E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();


Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5846,7 +5846,9 @@
   // pointers.  Everything else should be possible.
 
   QualType SrcTy = Src.get()->getType();
-  if (Context.hasSameUnqualifiedType(SrcTy, DestTy))
+  // We can use a no-op cast if the types are the same, or only adding/removing
+  // const, volatile, or restricted qualifiers.
+  if (Context.hasCvrSimilarType(SrcTy, DestTy))
 return CK_NoOp;
 
   switch (Type::ScalarTypeKind SrcKind = SrcTy->getScalarTypeKind()) {
@@ -7762,7 +7764,12 @@
 if (isa(RHSType)) {
   LangAS AddrSpaceL = LHSPointer->getPointeeType().getAddressSpace();
   LangAS AddrSpaceR = RHSType->getPointeeType().getAddressSpace();
-  Kind = AddrSpaceL != AddrSpaceR ? CK_AddressSpaceConversion : CK_BitCast;
+  if (AddrSpaceL != AddrSpaceR)
+Kind = CK_AddressSpaceConversion;
+  else if (Context.hasCvrSimilarType(RHSType, LHSType))
+Kind = CK_NoOp;
+  else
+Kind = CK_BitCast;
   return checkPointerTypesForAssignment(*this, LHSType, RHSType);
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -5861,11 +5861,7 @@
 // permitted in constant expressions in C++11. Bitcasts from cv void* are
 // also static_casts, but we disallow them as a resolution to DR1312.
 if (!E->getType()->isVoidPointerType()) {
-  // If we changed anything other than cvr-qualifiers, we can't use this
-  // value for constant folding. FIXME: Qualification conversions should
-  // always be CK_NoOp, but we get this wrong in C.
-  if (!Info.Ctx.hasCvrSimilarType(E->getType(), E->getSubExpr()->getType()))
-Result.Designator.setInvalid();
+  Result.Designator.setInvalid();
   if (SubExpr->getType()->isVoidPointerType())
 CCEDiag(E, diag::note_constexpr_invalid_cast)
   << 3 << SubExpr->getType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.

[James Y Knight via Phabricator via cfe-commits]

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

And, since EM_OffsetFold is now unused, remove it.

While builtin_object_size intends to ignore the presence of
side-effects in its argument, the EM_OffsetFold mode was NOT
configured to ignore side-effects. Rather it was effectively identical
to EM_ConstantFold -- its explanatory comment
notwithstanding.

However, currently, keepEvaluatingAfterSideEffect() is not always
honored -- sometimes evaluation continues despite it returning
false. Therefore, since the b_o_s code was only checking the return
value from evaluation, and not additionally checking the
HasSideEffects flag, side-effects _were_ in many cases actually being
ignored.

This change is a prerequisite cleanup towards fixing that issue.


https://reviews.llvm.org/D52924

Files:
  clang/lib/AST/ExprConstant.cpp


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -759,18 +759,6 @@
   /// context we try to fold them immediately since the optimizer never
   /// gets a chance to look at it.
   EM_PotentialConstantExpressionUnevaluated,
-
-  /// Evaluate as a constant expression. In certain scenarios, if:
-  /// - we find a MemberExpr with a base that can't be evaluated, or
-  /// - we find a variable initialized with a call to a function that has
-  ///   the alloc_size attribute on it
-  /// then we may consider evaluation to have succeeded.
-  ///
-  /// In either case, the LValue returned shall have an invalid base; in 
the
-  /// former, the base will be the invalid MemberExpr, in the latter, the
-  /// base will be either the alloc_size CallExpr or a CastExpr wrapping
-  /// said CallExpr.
-  EM_OffsetFold,
 } EvalMode;
 
 /// Are we checking whether the expression is a potential constant
@@ -874,7 +862,6 @@
   case EM_PotentialConstantExpression:
   case EM_ConstantExpressionUnevaluated:
   case EM_PotentialConstantExpressionUnevaluated:
-  case EM_OffsetFold:
 HasActiveDiagnostic = false;
 return OptionalDiagnostic();
   }
@@ -966,7 +953,6 @@
   case EM_ConstantExpression:
   case EM_ConstantExpressionUnevaluated:
   case EM_ConstantFold:
-  case EM_OffsetFold:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -985,7 +971,6 @@
   case EM_EvaluateForOverflow:
   case EM_IgnoreSideEffects:
   case EM_ConstantFold:
-  case EM_OffsetFold:
 return true;
 
   case EM_PotentialConstantExpression:
@@ -1021,7 +1006,6 @@
   case EM_ConstantExpressionUnevaluated:
   case EM_ConstantFold:
   case EM_IgnoreSideEffects:
-  case EM_OffsetFold:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -1093,18 +1077,18 @@
 }
   };
 
-  /// RAII object used to treat the current evaluation as the correct pointer
-  /// offset fold for the current EvalMode
-  struct FoldOffsetRAII {
+  /// RAII object used to set the current evaluation mode to ignore
+  /// side-effects.
+  struct IgnoreSideEffectsRAII {
 EvalInfo 
 EvalInfo::EvaluationMode OldMode;
-explicit FoldOffsetRAII(EvalInfo )
+explicit IgnoreSideEffectsRAII(EvalInfo )
 : Info(Info), OldMode(Info.EvalMode) {
   if (!Info.checkingPotentialConstantExpression())
-Info.EvalMode = EvalInfo::EM_OffsetFold;
+Info.EvalMode = EvalInfo::EM_IgnoreSideEffects;
 }
 
-~FoldOffsetRAII() { Info.EvalMode = OldMode; }
+~IgnoreSideEffectsRAII() { Info.EvalMode = OldMode; }
   };
 
   /// RAII object used to optionally suppress diagnostics and side-effects from
@@ -8049,7 +8033,7 @@
 // If there are any, but we can determine the pointed-to object anyway, 
then
 // ignore the side-effects.
 SpeculativeEvaluationRAII SpeculativeEval(Info);
-FoldOffsetRAII Fold(Info);
+IgnoreSideEffectsRAII Fold(Info);
 
 if (E->isGLValue()) {
   // It's possible for us to be given GLValues if we're called via
@@ -8117,7 +8101,6 @@
 case EvalInfo::EM_ConstantFold:
 case EvalInfo::EM_EvaluateForOverflow:
 case EvalInfo::EM_IgnoreSideEffects:
-case EvalInfo::EM_OffsetFold:
   // Leave it to IR generation.
   return Error(E);
 case EvalInfo::EM_ConstantExpressionUnevaluated:


Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -759,18 +759,6 @@
   /// context we try to fold them immediately since the optimizer never
   /// gets a chance to look at it.
   EM_PotentialConstantExpressionUnevaluated,
-
-  /// Evaluate as a constant expression. In certain scenarios, if:
-  /// - we find a MemberExpr with a base that can't be evaluated, or
-  /// - we 

[PATCH] D52926: Rename EM_ConstantExpressionUnevaluated to EM_ConstantFoldUnevaluated, which is actually what the code is currently doing.

[James Y Knight via Phabricator via cfe-commits]

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

And, configure it to explicitly request equivalent behavior to
ConstantFold, instead of getting that behavior accidentally.

While it seems that diagnose_if and enable_if were intended to require
a constexpr argument, the code was actually only checking whether it
could be constant-folded, due to the confusing way the constant
evaluator code currently works.

It _probably_ should be fixed to actually check for
constexpr-ness. However, r290584 now depends on it NOT doing so -- and
it's possible that some users are, too.

I'd like to land this, to unblock the follow-on cleanup -- without
changing the semantics first -- and maybe loop back to convert it to
require a constexpr later, if it looks like users aren't depending on
the semantics it has now.

This change is a prerequisite for fixing the confusion in the
ExprConstant code which led to this situation.


https://reviews.llvm.org/D52926

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/Sema/enable_if.c

Index: clang/test/Sema/enable_if.c
===
--- clang/test/Sema/enable_if.c
+++ clang/test/Sema/enable_if.c
@@ -170,4 +170,24 @@
   variadic_enable_if(0, m); // expected-error{{no matching}}
   variadic_enable_if(0, m, 3); // expected-error{{no matching}}
 }
+
+// Test of the current weird semantics of enable_if (and diagnose_if).
+//
+// There's two kinds of checking of the expression going on:
+//  1. Whether it could potentially be a constexpr -- without access to the
+// parameters.
+//  2. Checking if the condition evaluates to true with concrete
+// arguments. While #1 requires that it could potentially be a constexpr,
+// this step does not require that it actually _IS_ a constexpr, only that
+// it's evaluable! (Note: this looseness might be unintended)
+
+void non_constexpr() __attribute__((enable_if((int*)(char*)0 == 0, ""))); // expected-error{{'enable_if' attribute expression never produces a constant expression}} expected-note{{cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression}}
+void non_constexpr_call() {
+non_constexpr();
+}
+
+void non_constexpr_conditional(long x) __attribute__((enable_if(x?1: (int*)(char*)0 == 0, "")));
+void non_constexpr_conditional_call() {
+non_constexpr_conditional(0);
+}
 #endif
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -744,12 +744,11 @@
   /// can't be modeled.
   EM_IgnoreSideEffects,
 
-  /// Evaluate as a constant expression. Stop if we find that the expression
-  /// is not a constant expression. Some expressions can be retried in the
+  /// Evaluate as a constant fold. Some expressions can be retried in the
   /// optimizer if we don't constant fold them here, but in an unevaluated
-  /// context we try to fold them immediately since the optimizer never
-  /// gets a chance to look at it.
-  EM_ConstantExpressionUnevaluated,
+  /// context we try to fold them immediately since the optimizer never gets
+  /// a chance to look at it.
+  EM_ConstantFoldUnevaluated,
 
   /// Evaluate as a potential constant expression. Keep going if we hit a
   /// construct that we can't evaluate yet (because we don't yet know the
@@ -854,13 +853,13 @@
   case EM_ConstantFold:
   case EM_IgnoreSideEffects:
   case EM_EvaluateForOverflow:
+  case EM_ConstantFoldUnevaluated:
 if (!HasFoldFailureDiagnostic)
   break;
 // We've already failed to fold something. Keep that diagnostic.
 LLVM_FALLTHROUGH;
   case EM_ConstantExpression:
   case EM_PotentialConstantExpression:
-  case EM_ConstantExpressionUnevaluated:
   case EM_PotentialConstantExpressionUnevaluated:
 HasActiveDiagnostic = false;
 return OptionalDiagnostic();
@@ -951,7 +950,7 @@
 return true;
 
   case EM_ConstantExpression:
-  case EM_ConstantExpressionUnevaluated:
+  case EM_ConstantFoldUnevaluated:
   case EM_ConstantFold:
 return false;
   }
@@ -971,12 +970,12 @@
   case EM_EvaluateForOverflow:
   case EM_IgnoreSideEffects:
   case EM_ConstantFold:
+  case EM_ConstantFoldUnevaluated:
 return true;
 
   case EM_PotentialConstantExpression:
   case EM_PotentialConstantExpressionUnevaluated:
   case EM_ConstantExpression:
-  case EM_ConstantExpressionUnevaluated:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -1003,7 +1002,7 @@
 return true;
 
   case EM_ConstantExpression:
-  case EM_ConstantExpressionUnevaluated:
+  case EM_ConstantFoldUnevaluated:
   case EM_ConstantFold:
   case EM_IgnoreSideEffects:

[PATCH] D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC344110: ExprConstant: Make __builtin_object_size use 
EM_IgnoreSideEffects. (authored by jyknight, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D52924?vs=168435=168935#toc

Repository:
  rC Clang

https://reviews.llvm.org/D52924

Files:
  lib/AST/ExprConstant.cpp


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -759,18 +759,6 @@
   /// context we try to fold them immediately since the optimizer never
   /// gets a chance to look at it.
   EM_PotentialConstantExpressionUnevaluated,
-
-  /// Evaluate as a constant expression. In certain scenarios, if:
-  /// - we find a MemberExpr with a base that can't be evaluated, or
-  /// - we find a variable initialized with a call to a function that has
-  ///   the alloc_size attribute on it
-  /// then we may consider evaluation to have succeeded.
-  ///
-  /// In either case, the LValue returned shall have an invalid base; in 
the
-  /// former, the base will be the invalid MemberExpr, in the latter, the
-  /// base will be either the alloc_size CallExpr or a CastExpr wrapping
-  /// said CallExpr.
-  EM_OffsetFold,
 } EvalMode;
 
 /// Are we checking whether the expression is a potential constant
@@ -874,7 +862,6 @@
   case EM_PotentialConstantExpression:
   case EM_ConstantExpressionUnevaluated:
   case EM_PotentialConstantExpressionUnevaluated:
-  case EM_OffsetFold:
 HasActiveDiagnostic = false;
 return OptionalDiagnostic();
   }
@@ -966,7 +953,6 @@
   case EM_ConstantExpression:
   case EM_ConstantExpressionUnevaluated:
   case EM_ConstantFold:
-  case EM_OffsetFold:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -985,7 +971,6 @@
   case EM_EvaluateForOverflow:
   case EM_IgnoreSideEffects:
   case EM_ConstantFold:
-  case EM_OffsetFold:
 return true;
 
   case EM_PotentialConstantExpression:
@@ -1021,7 +1006,6 @@
   case EM_ConstantExpressionUnevaluated:
   case EM_ConstantFold:
   case EM_IgnoreSideEffects:
-  case EM_OffsetFold:
 return false;
   }
   llvm_unreachable("Missed EvalMode case");
@@ -1093,18 +1077,18 @@
 }
   };
 
-  /// RAII object used to treat the current evaluation as the correct pointer
-  /// offset fold for the current EvalMode
-  struct FoldOffsetRAII {
+  /// RAII object used to set the current evaluation mode to ignore
+  /// side-effects.
+  struct IgnoreSideEffectsRAII {
 EvalInfo 
 EvalInfo::EvaluationMode OldMode;
-explicit FoldOffsetRAII(EvalInfo )
+explicit IgnoreSideEffectsRAII(EvalInfo )
 : Info(Info), OldMode(Info.EvalMode) {
   if (!Info.checkingPotentialConstantExpression())
-Info.EvalMode = EvalInfo::EM_OffsetFold;
+Info.EvalMode = EvalInfo::EM_IgnoreSideEffects;
 }
 
-~FoldOffsetRAII() { Info.EvalMode = OldMode; }
+~IgnoreSideEffectsRAII() { Info.EvalMode = OldMode; }
   };
 
   /// RAII object used to optionally suppress diagnostics and side-effects from
@@ -8049,7 +8033,7 @@
 // If there are any, but we can determine the pointed-to object anyway, 
then
 // ignore the side-effects.
 SpeculativeEvaluationRAII SpeculativeEval(Info);
-FoldOffsetRAII Fold(Info);
+IgnoreSideEffectsRAII Fold(Info);
 
 if (E->isGLValue()) {
   // It's possible for us to be given GLValues if we're called via
@@ -8117,7 +8101,6 @@
 case EvalInfo::EM_ConstantFold:
 case EvalInfo::EM_EvaluateForOverflow:
 case EvalInfo::EM_IgnoreSideEffects:
-case EvalInfo::EM_OffsetFold:
   // Leave it to IR generation.
   return Error(E);
 case EvalInfo::EM_ConstantExpressionUnevaluated:


Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -759,18 +759,6 @@
   /// context we try to fold them immediately since the optimizer never
   /// gets a chance to look at it.
   EM_PotentialConstantExpressionUnevaluated,
-
-  /// Evaluate as a constant expression. In certain scenarios, if:
-  /// - we find a MemberExpr with a base that can't be evaluated, or
-  /// - we find a variable initialized with a call to a function that has
-  ///   the alloc_size attribute on it
-  /// then we may consider evaluation to have succeeded.
-  ///
-  /// In either case, the LValue returned shall have an invalid base; in the
-  /// former, the base will be the invalid MemberExpr, in the latter, the
-  /// base will be either the alloc_size CallExpr or a CastExpr wrapping
-  /// said CallExpr.
-  EM_OffsetFold,
  

[PATCH] D52703: Allow ifunc resolvers to accept arguments

[James Y Knight via Phabricator via cfe-commits]

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

Given that there's no technical reason for the compiler to prohibit this (it 
was just clang trying to diagnose a probable user-error, which turns out to not 
be as probable as ), this seems like the right solution to me.


https://reviews.llvm.org/D52703



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D54355#1328557 , @void wrote:

> The issue is that "`n`" is expecting an immediate value, but the result of 
> the ternary operator isn't calculated by the front-end, because it doesn't 
> "know" that the evaluation of `__builtin_constant_p` shouldn't be delayed (it 
> being compiled at `-O0`). I suspect that this issue will also happen with the 
> "`i`" constraint.


Indeed. We _do_ actually already know that in clang too, however -- clang 
already knows that "n" requires an immediate, and does constant expression 
evaluation for it, e.g. to expand constexpr functions. Grep for 
"requiresImmediateConstant". I guess it's just not hooked up to the __b_c_p 
correctly, though.

(Note, as a workaround, you could use `|` instead of `||`, or put the 
expression as the value of an enumeration constant).


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

That example cannot be expected to ever evaluate the expression as "1" -- it 
doesn't in GCC, nor should it in Clang. An asm constraint of "n" or "i" (but 
not, e.g., "nr") must require a constant expression, and evaluating the 
argument as a constant expression necessarily means always evaluating it to -1.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

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

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

This seems like a good start, but not complete.

"n" and "i" both should require that their argument is a constant expression. 
For "n", it actually must be an immediate constant integer, so 
setRequiresImmediate() should be used there. For "i", you may use an lvalue 
constant as well. The way we seem to indicate that, today, is with `if 
(!Info.allowsRegister() && !Info.allowsMemory())`. However the code in that 
block does not today *require* that the evaluation as a constant succeed...and 
it should.

It should also only require that the result is an integer when 
`requiresImmediateConstant()`.

Additionally, the Sema code needs to have the same conditions as the CodeGen 
code for when a constant expression is required, but it doesn't. (before or 
after your change).


Repository:
  rC Clang

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

https://reviews.llvm.org/D55616



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

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D54355#1327237 , @craig.topper 
wrote:

> Here's the test case that we have https://reviews.llvm.org/P8123   gcc seems 
> to accept it at O0


Smaller test case:

  extern unsigned long long __sdt_unsp;
  
  void foo() {
__asm__ __volatile__(
"" :: "n"( (__builtin_constant_p(__sdt_unsp) || 0) ? 1 : -1));
  }


Repository:
  rC Clang

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

https://reviews.llvm.org/D54355



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

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Headers/__stddef_max_align_t.h:40
   __attribute__((__aligned__(__alignof__(long double;
+#ifdef __i386__
+  __float128 __clang_max_align_nonce3

Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in 
InitPreprocessor alongside the rest of the SIZEOF macros?

And then use that to determine whether to add float128 to the union? This 
change, as is, will break on any target which is i386 but doesn't define 
__float128, e.g. i386-unknown-unknown.

The only additional target which will be modified with that (that is: the only 
other target which has a float128 type, but for which max_align isn't already 
16) is systemz-*-linux.

But I think that's actually indicating a pre-existing bug in the SystemZ config 
-- it's configured for a 16-byte long double, with 8-byte alignment, but the 
ABI doc seems to call for 16-byte alignment. +Ulrich for comment on that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55057



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

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Headers/__stddef_max_align_t.h:40
   __attribute__((__aligned__(__alignof__(long double;
+#ifdef __i386__
+  __float128 __clang_max_align_nonce3

uweigand wrote:
> jyknight wrote:
> > Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in 
> > InitPreprocessor alongside the rest of the SIZEOF macros?
> > 
> > And then use that to determine whether to add float128 to the union? This 
> > change, as is, will break on any target which is i386 but doesn't define 
> > __float128, e.g. i386-unknown-unknown.
> > 
> > The only additional target which will be modified with that (that is: the 
> > only other target which has a float128 type, but for which max_align isn't 
> > already 16) is systemz-*-linux.
> > 
> > But I think that's actually indicating a pre-existing bug in the SystemZ 
> > config -- it's configured for a 16-byte long double, with 8-byte alignment, 
> > but the ABI doc seems to call for 16-byte alignment. +Ulrich for comment on 
> > that.
> That's a bug in the ABI doc which we'll fix once we get around to releasing 
> an updated version.
> 
> long double on SystemZ must be 8-byte aligned, which is the maximum alignment 
> of all standard types on Z, and this was chosen because historically the ABI 
> only guarantees an 8-byte stack alignment, so 16-byte aligned standard types 
> would be awkward.
Then perhaps it's a bug that `__alignof__(__float128)` returns 16 with `-target 
systemz-linux`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55057



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

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: rsmith, chandlerc.
Herald added a subscriber: arphaman.

This warning, Wexperimental-driver-option, is on by default, exempt
from -Werror, and may be disabled.

Additionally, change the documentation to note that these options are
not intended for common usage.


https://reviews.llvm.org/D55150

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3419,6 +3419,11 @@
   if (options & CXTranslationUnit_KeepGoing)
 Diags->setSuppressAfterFatalError(false);
 
+  // Suppress driver warning for use of -Xclang, since we add it ourselves.
+  Diags->setSeverityForGroup(diag::Flavor::WarningOrError,
+ "experimental-driver-option",
+ diag::Severity::Ignored, SourceLocation());
+
   // Recover resources if we crash before exiting this function.
   llvm::CrashRecoveryContextCleanupRegistrar >
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4994,6 +4994,11 @@
 
   // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option
   // parser.
+  if (Args.hasArg(options::OPT_Xclang))
+D.Diag(diag::warn_drv_xclang_option);
+  if (Args.hasArg(options::OPT_mllvm))
+D.Diag(diag::warn_drv_mllvm_option);
+
   // -finclude-default-header flag is for preprocessor,
   // do not pass it to other cc1 commands when save-temps is enabled
   if (C.getDriver().isSaveTempsEnabled() &&
@@ -5926,6 +5931,8 @@
   CollectArgsForIntegratedAssembler(C, Args, CmdArgs,
 getToolChain().getDriver());
 
+  if (Args.hasArg(options::OPT_mllvm))
+D.Diag(diag::warn_drv_mllvm_option);
   Args.AddAllArgs(CmdArgs, options::OPT_mllvm);
 
   assert(Output.isFilename() && "Unexpected lipo output.");
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -466,7 +466,7 @@
   HelpText<"Pass  to the assembler">, MetaVarName<"">,
   Group;
 def Xclang : Separate<["-"], "Xclang">,
-  HelpText<"Pass  to the clang compiler">, MetaVarName<"">,
+  HelpText<"Pass  to the clang cc1 frontend. For experimental use only">, MetaVarName<"">,
   Flags<[DriverOption, CoreOption]>, Group;
 def Xcuda_fatbinary : Separate<["-"], "Xcuda-fatbinary">,
   HelpText<"Pass  to fatbinary invocation">, MetaVarName<"">;
@@ -2002,7 +2002,7 @@
 def mlinker_version_EQ : Joined<["-"], "mlinker-version=">,
   Flags<[DriverOption]>;
 def mllvm : Separate<["-"], "mllvm">, Flags<[CC1Option,CC1AsOption,CoreOption]>,
-  HelpText<"Additional arguments to forward to LLVM's option processing">;
+  HelpText<"Additional arguments to forward to LLVM's option processing. For experimental use only">;
 def mmacosx_version_min_EQ : Joined<["-"], "mmacosx-version-min=">,
   Group, HelpText<"Set Mac OS X deployment target">;
 def mmacos_version_min_EQ : Joined<["-"], "mmacos-version-min=">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1039,3 +1039,7 @@
 // A warning group specifically for warnings related to function
 // multiversioning.
 def FunctionMultiVersioning : DiagGroup<"function-multiversion">;
+
+// Warning for command-line options that are not a supported part of the clang command-line interface.
+// Warnings in this group should be on by default, and exempt from -Werror.
+def ExperimentalDriverOption : DiagGroup<"experimental-driver-option">;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -405,4 +405,14 @@
 def warn_drv_moutline_unsupported_opt : Warning<
   "The '%0' architecture does not support -moutline; flag ignored">,
   InGroup;
+
+def warn_drv_xclang_option : Warning<
+  "-Xclang options are for experimental use only; future compatibility may not be preserved">,
+  InGroup, DefaultWarnNoWerror;
+
+def warn_drv_mllvm_option : Warning<
+  "-mllvm options are for experimental use only; future compatibility may not be preserved">,
+  InGroup, DefaultWarnNoWerror;
+
 }
+
Index: clang/docs/UsersManual.rst
===
--- 

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1318082 , @chandlerc wrote:

> I think this should be `internal-driver-option` and the text updated? I don't 
> think these are necessarily experimental, they're internals of the compiler 
> implementation, and not a supported interface for users. Unsure how to best 
> explain this.


Yea, I was trying to figure out a good name. I thought "unsupported" at first, 
but that is a tricky word, since it has the connotation of "doesn't work in 
this version", which is not what I mean to imply -- the thing someone wants to 
do likely does work, but we need to indicate no guarantees about the future. 
Which is where "experimental" came from -- the options are useful for 
experimenting with compiler features that are not fully baked.

Perhaps "internal" is okay too...


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

https://reviews.llvm.org/D55150



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

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1321829 , @efriedma wrote:

> I'm not sure that putting a warning that can be disabled really helps here; 
> anyone who needs the option will just disable the warning anyway, and then 
> users adding additional options somewhere else in the build system will miss 
> the warning.
>
> Instead, it would probably be better to rename Xclang and mllvm to something 
> that makes it clear the user is doing something unsupported. Maybe 
> "--unstable-llvm-option" and "--unstable-clang-option" or something like 
> that.  (This will lead to some breakage, but the breakage is roughly 
> equivalent for anyone using -Werror.)


Possibly you missed that this flag is exempt from -Werror -- it _only_ prints a 
warning, and is not an error, even when -Werror is specified.


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

https://reviews.llvm.org/D55150



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

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1321759 , @george.karpenkov 
wrote:

> Using `-Xclang` is the only way to pass options to the static analyzer, I 
> don't think we should warn on it.


Well,, that seems unfortunate if we have the only supported interface for the 
static analyzer be an internal interface. Perhaps it can be given a different 
option? Even discounting this change, I that seems like it would be appropriate.

In D55150#1321771 , @arphaman wrote:

> Swift uses `-Xclang` to pass in build settings to its own build and to pass 
> in custom options through its Clang importer that we intentionally don't want 
> to expose to Clang's users. We don't want to warn for those uses for sure.


I'm not sure if I understand correctly, so I'll try to reiterate: Swift calls 
clang internally, and when it does so, intentionally uses options that are not 
generally intended for others to use. Of course we shouldn't emit warnings in 
that case.

Is that a correct understanding? If so, doesn't it just make sense for that 
constructed command-line to disable the warning? And, isn't it good that it 
will warn, otherwise, in order to discourage people from using those flags that 
are intentionally-not-exposed?


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

https://reviews.llvm.org/D55150



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

[PATCH] D54547: PTH-- Remove feature entirely-

[James Y Knight via Phabricator via cfe-commits]

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

Since you've already submitted a fix to Boost, 
https://github.com/boostorg/build/commit/3385fe2aa699a45e722a1013658f824b6a7c761f,
 I think this is fine to remove.


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

https://reviews.llvm.org/D54547



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

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Headers/__stddef_max_align_t.h:40
   __attribute__((__aligned__(__alignof__(long double;
+#ifdef __i386__
+  __float128 __clang_max_align_nonce3

EricWF wrote:
> uweigand wrote:
> > jyknight wrote:
> > > uweigand wrote:
> > > > jyknight wrote:
> > > > > Can you fix clang to consistently define `__SIZEOF_FLOAT128__` in 
> > > > > InitPreprocessor alongside the rest of the SIZEOF macros?
> > > > > 
> > > > > And then use that to determine whether to add float128 to the union? 
> > > > > This change, as is, will break on any target which is i386 but 
> > > > > doesn't define __float128, e.g. i386-unknown-unknown.
> > > > > 
> > > > > The only additional target which will be modified with that (that is: 
> > > > > the only other target which has a float128 type, but for which 
> > > > > max_align isn't already 16) is systemz-*-linux.
> > > > > 
> > > > > But I think that's actually indicating a pre-existing bug in the 
> > > > > SystemZ config -- it's configured for a 16-byte long double, with 
> > > > > 8-byte alignment, but the ABI doc seems to call for 16-byte 
> > > > > alignment. +Ulrich for comment on that.
> > > > That's a bug in the ABI doc which we'll fix once we get around to 
> > > > releasing an updated version.
> > > > 
> > > > long double on SystemZ must be 8-byte aligned, which is the maximum 
> > > > alignment of all standard types on Z, and this was chosen because 
> > > > historically the ABI only guarantees an 8-byte stack alignment, so 
> > > > 16-byte aligned standard types would be awkward.
> > > Then perhaps it's a bug that `__alignof__(__float128)` returns 16 with 
> > > `-target systemz-linux`?
> > Huh, really __float128 should not be supported at all on SystemZ.  It's not 
> > supported with GCC either (GCC never provides __float128 on targets where 
> > long double is already IEEE-128).  (GCC does support the new _Float128 on 
> > SystemZ, but that's not yet supported by clang anywhere.)
> > 
> > If it were supported, I agree it should be an alias for long double, and 
> > also have an alignof of 8.
> @jyknight Ack. I'll add `__SIZEOF_FLOAT128__`.
@uweigand : One of those fixes needs to land before this, so that systemz's 
max_align_t doesn't change to 16 in the meantime. I think your preference would 
be for it to be simply removed, right? Looks like the type was originally added 
in https://reviews.llvm.org/D19125 -- possibly in error, since the focus was 
x86_64.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55057



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

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1321046 , @kristina wrote:

> Personally I'm against this type of warning as it's likely anyone using 
> `-mllvm` is actually intending to adjust certain behaviors across one or more 
> passes with a lot of switches supported by it being intentionally hidden from 
> ` --help` output requiring explicitly specifying that hidden flags 
> be shown.


There is a cost to having people encode these flags into their build systems -- 
it can then cause issues if we ever change these internal flags. I do not think 
any Clang maintainer intends to support these as stable APIs, unlike most of 
the driver command-line. But, neither -Xclang nor -mllvm obviously scream out 
"don't use this!", and so people may then add them to their buildsystems 
without causing reviewers to say "Wait...really? Are you sure that's a good 
idea?".

That's why I think a warning is useful -- it'll discourage people from using 
them when they haven't properly understand the consequences, but does not 
prevent them from doing so, when they actually do.

> For example, I routinely use the following with SEH (excuse some of the 
> naming, this is just a downstream fork however):
>  `-mllvm -target-enable-seh=true -mllvm -force-msvc-seh=true -mllvm 
> -wtfabi-opts=0x1EF77F`

If you already are passing that, do you see a problem with instead passing
 `-mllvm -target-enable-seh=true -mllvm -force-msvc-seh=true -mllvm 
-wtfabi-opts=0x1EF77F -Wno-experimental-driver-option`
?


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

https://reviews.llvm.org/D55150



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

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D55150#1321705 , @kristina wrote:

> > There is a cost to having people encode these flags into their build 
> > systems -- it can then cause issues if we ever change these internal flags. 
> > I do not think any Clang maintainer intends to support these as stable 
> > APIs, unlike most of the driver command-line. But, neither -Xclang nor 
> > -mllvm obviously scream out "don't use this!", and so people may then add 
> > them to their buildsystems without causing reviewers to say "Wait...really? 
> > Are you sure that's a good idea?".
>
> This is why I proposed a compromise, allow this warning to be disabled 
> completely for people actively using those flags, which are pretty much 
> exclusively toolchain developers (well basically what I proposed, since it's 
> not clear what counts as a build made by someone who is working and debugging 
> a pass, being fully aware of those flags, using the subset of them specific 
> to that pass/feature, I would say assertion+dump enabled builds are closest, 
> but having an explicit build flag for it would be nicer). It's more unified 
> than everyone either adding workarounds into build systems or disabling it 
> completely (by just removing it).


I mean, I'm not much opposed to adding that -- just that any new build-time 
options is always a minor shame. But you didn't respond to the other part of my 
message -- is adding `-Wno-experimental-driver-option` to your compile-flags 
not already a completely sufficient solution for your use-case?

> Besides, I don't think this really ever surfaced as an issue, until 
> Chandler's idea with regards to an unsupressable warning for performance 
> tests for 7.x.x

The primary impetus for me was actually the discovery that Boost's build system 
was using "-Xclang -include-pth" a few weeks back.


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

https://reviews.llvm.org/D55150



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

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ping.


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

https://reviews.llvm.org/D53199



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

[PATCH] D57330: Adjust documentation for git migration.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: jlebar, rnk, mehdi_amini.
Herald added subscribers: jsji, jfb, arphaman, christof, delcypher, hiraditya, 
nhaehnle, jvesely, nemanjai, kubamracek, arsenm.
Herald added a reviewer: bollu.
Herald added a reviewer: serge-sans-paille.

This fixes most references to the paths:
 llvm.org/svn/
 llvm.org/git/
 llvm.org/viewvc/
 github.com/llvm-mirror/
 github.com/llvm-project/
 reviews.llvm.org/diffusion/

to instead point to https://github.com/llvm/llvm-project.

This is *not* a trivial substitution, because additionally, all the
checkout instructions had to be migrated to instruct users on how to
use the monorepo layout, setting LLVM_ENABLE_PROJECTS instead of
checking out various projects into various subdirectories.

I've attempted to *NOT* change any scripts here, only
documentation. The scripts will have to be addressed separately.


https://reviews.llvm.org/D57330

Files:
  clang-tools-extra/docs/clang-rename.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/Integrations.rst
  clang/.gitignore
  clang/docs/ClangPlugins.rst
  clang/docs/ClangTools.rst
  clang/docs/ControlFlowIntegrityDesign.rst
  clang/docs/InternalsManual.rst
  clang/docs/LibASTMatchersTutorial.rst
  clang/docs/LibTooling.rst
  clang/docs/Toolchain.rst
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/www/analyzer/checker_dev_manual.html
  clang/www/get_started.html
  clang/www/hacking.html
  clang/www/menu.html.incl
  compiler-rt/include/sanitizer/tsan_interface_atomic.h
  compiler-rt/lib/tsan/rtl/tsan_interface.h
  compiler-rt/www/index.html
  compiler-rt/www/menu.html.incl
  libclc/www/index.html
  libcxx/docs/BuildingLibcxx.rst
  libcxx/docs/index.rst
  libcxx/www/TS_deprecation.html
  libcxx/www/atomic_design.html
  libcxx/www/atomic_design_a.html
  libcxx/www/atomic_design_b.html
  libcxx/www/atomic_design_c.html
  libcxx/www/cxx1y_status.html
  libcxx/www/cxx1z_status.html
  libcxx/www/cxx2a_status.html
  libcxx/www/index.html
  libcxx/www/ts1z_status.html
  libcxx/www/type_traits_design.html
  libcxx/www/upcoming_meeting.html
  libcxxabi/www/index.html
  libunwind/docs/BuildingLibunwind.rst
  libunwind/docs/index.rst
  lld/docs/getting_started.rst
  lld/docs/index.rst
  lldb/docs/building-with-debug-llvm.txt
  
lldb/packages/Python/lldbsuite/test/functionalities/command_source/TestCommandSource.py
  
lldb/packages/Python/lldbsuite/test/lang/objc/objc-optimized/TestObjcOptimized.py
  lldb/utils/vim-lldb/doc/lldb.txt
  lldb/www/adding-language-support.html
  lldb/www/build.html
  lldb/www/index.html
  lldb/www/python-reference.html
  lldb/www/scripting.html
  lldb/www/sidebar.incl
  lldb/www/source.html
  lldb/www/symbolication.html
  lldb/www/varformats.html
  llgo/README.TXT
  llvm/docs/CompileCudaWithLLVM.rst
  llvm/docs/LibFuzzer.rst
  llvm/docs/TestSuiteMakefileGuide.rst
  llvm/docs/TestingGuide.rst
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/test/CodeGen/PowerPC/pr24546.ll
  llvm/utils/lit/setup.py
  openmp/www/index.html
  polly/docs/TipsAndTricks.rst
  polly/www/get_started.html
  polly/www/menu.html.incl
  polly/www/todo.html

Index: polly/www/todo.html
===
--- polly/www/todo.html
+++ polly/www/todo.html
@@ -411,12 +411,6 @@
 >http://llvm.org/svn/llvm-project/polly
  Tobias
 
-
-
- Git mirror
-
-git://llvm.org/git/polly.git
- Tobias
 
 
  Commit mails
Index: polly/www/menu.html.incl
===
--- polly/www/menu.html.incl
+++ polly/www/menu.html.incl
@@ -34,7 +34,7 @@
 http://lab.llvm.org:8080/coverage/coverage-reports/polly/index.html;>Code Coverage
 http://llvm.org/reports/scan-build/;>Static analysis
 Doxygen
-https://github.com/llvm-mirror/polly;>Source @ GitHub
+https://github.com/llvm/llvm-project/tree/master/polly;>Source @ GitHub
   
 
   
Index: polly/www/get_started.html
===
--- polly/www/get_started.html
+++ polly/www/get_started.html
@@ -33,20 +33,14 @@
  Manual 
  Get the code 
 
-Warning: Polly/LLVM/clang need to be checked out at the same time.
-
 
-git clone http://llvm.org/git/llvm.git llvm_git
-git clone http://llvm.org/git/polly.git llvm_git/tools/polly
-
-# Also build the matching clang-version (optional)
-git clone http://llvm.org/git/clang.git llvm_git/tools/clang
+git clone http://github.com/llvm/llvm-project.git llvm_git
 
 Build Polly
 
 
 mkdir llvm_build && cd llvm_build
-cmake ../llvm_git && make
+cmake -DLLVM_ENABLE_PROJECTS='polly;clang' ../llvm_git/llvm && make
 
 
  Test Polly
@@ -59,7 +53,7 @@
 build. To configure Polly to use a pre-built LLVM, set the
 -DCMAKE_PREFIX_PATH option:
 
-cmake -DCMAKE_PREFIX_PATH=${LLVM_PREFIX}/lib/cmake/llvm
+cmake -DCMAKE_PREFIX_PATH=${LLVM_PREFIX}/lib/cmake/llvm ../llvm_git/polly
 
 To run unittests, 

[PATCH] D57330: Adjust documentation for git migration.

[James Y Knight via Phabricator via cfe-commits]

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

Fix some warnings I added.

Restore TestSuiteMakefileGuide.rst, which apparently isn't 100%
obsolete. (But it is incorrect, and I'm not sure exactly how to fix
it, so I just left a FIXME).


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

https://reviews.llvm.org/D57330

Files:
  clang-tools-extra/docs/clang-rename.rst
  clang-tools-extra/docs/clang-tidy/Contributing.rst
  clang-tools-extra/docs/clang-tidy/Integrations.rst
  clang/.gitignore
  clang/docs/ClangPlugins.rst
  clang/docs/ClangTools.rst
  clang/docs/ControlFlowIntegrityDesign.rst
  clang/docs/InternalsManual.rst
  clang/docs/LibASTMatchersTutorial.rst
  clang/docs/LibTooling.rst
  clang/docs/Toolchain.rst
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/www/analyzer/checker_dev_manual.html
  clang/www/get_started.html
  clang/www/hacking.html
  clang/www/menu.html.incl
  compiler-rt/include/sanitizer/tsan_interface_atomic.h
  compiler-rt/lib/tsan/rtl/tsan_interface.h
  compiler-rt/www/index.html
  compiler-rt/www/menu.html.incl
  libclc/www/index.html
  libcxx/docs/BuildingLibcxx.rst
  libcxx/docs/index.rst
  libcxx/www/TS_deprecation.html
  libcxx/www/atomic_design.html
  libcxx/www/atomic_design_a.html
  libcxx/www/atomic_design_b.html
  libcxx/www/atomic_design_c.html
  libcxx/www/cxx1y_status.html
  libcxx/www/cxx1z_status.html
  libcxx/www/cxx2a_status.html
  libcxx/www/index.html
  libcxx/www/ts1z_status.html
  libcxx/www/type_traits_design.html
  libcxx/www/upcoming_meeting.html
  libcxxabi/www/index.html
  libunwind/docs/BuildingLibunwind.rst
  libunwind/docs/index.rst
  lld/docs/getting_started.rst
  lld/docs/index.rst
  lldb/docs/building-with-debug-llvm.txt
  
lldb/packages/Python/lldbsuite/test/functionalities/command_source/TestCommandSource.py
  
lldb/packages/Python/lldbsuite/test/lang/objc/objc-optimized/TestObjcOptimized.py
  lldb/utils/vim-lldb/doc/lldb.txt
  lldb/www/adding-language-support.html
  lldb/www/build.html
  lldb/www/index.html
  lldb/www/python-reference.html
  lldb/www/scripting.html
  lldb/www/sidebar.incl
  lldb/www/source.html
  lldb/www/symbolication.html
  lldb/www/varformats.html
  llgo/README.TXT
  llvm/docs/CompileCudaWithLLVM.rst
  llvm/docs/LibFuzzer.rst
  llvm/docs/TestSuiteGuide.md
  llvm/docs/TestSuiteMakefileGuide.rst
  llvm/docs/TestingGuide.rst
  llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/test/CodeGen/PowerPC/pr24546.ll
  llvm/utils/lit/setup.py
  openmp/www/index.html
  polly/docs/TipsAndTricks.rst
  polly/www/get_started.html
  polly/www/menu.html.incl
  polly/www/todo.html

Index: polly/www/todo.html
===
--- polly/www/todo.html
+++ polly/www/todo.html
@@ -411,12 +411,6 @@
 >http://llvm.org/svn/llvm-project/polly
  Tobias
 
-
-
- Git mirror
-
-git://llvm.org/git/polly.git
- Tobias
 
 
  Commit mails
Index: polly/www/menu.html.incl
===
--- polly/www/menu.html.incl
+++ polly/www/menu.html.incl
@@ -34,7 +34,7 @@
 http://lab.llvm.org:8080/coverage/coverage-reports/polly/index.html;>Code Coverage
 http://llvm.org/reports/scan-build/;>Static analysis
 Doxygen
-https://github.com/llvm-mirror/polly;>Source @ GitHub
+https://github.com/llvm/llvm-project/tree/master/polly;>Source @ GitHub
   
 
   
Index: polly/www/get_started.html
===
--- polly/www/get_started.html
+++ polly/www/get_started.html
@@ -33,20 +33,14 @@
  Manual 
  Get the code 
 
-Warning: Polly/LLVM/clang need to be checked out at the same time.
-
 
-git clone http://llvm.org/git/llvm.git llvm_git
-git clone http://llvm.org/git/polly.git llvm_git/tools/polly
-
-# Also build the matching clang-version (optional)
-git clone http://llvm.org/git/clang.git llvm_git/tools/clang
+git clone http://github.com/llvm/llvm-project.git llvm_git
 
 Build Polly
 
 
 mkdir llvm_build && cd llvm_build
-cmake ../llvm_git && make
+cmake -DLLVM_ENABLE_PROJECTS='polly;clang' ../llvm_git/llvm && make
 
 
  Test Polly
@@ -59,7 +53,7 @@
 build. To configure Polly to use a pre-built LLVM, set the
 -DCMAKE_PREFIX_PATH option:
 
-cmake -DCMAKE_PREFIX_PATH=${LLVM_PREFIX}/lib/cmake/llvm
+cmake -DCMAKE_PREFIX_PATH=${LLVM_PREFIX}/lib/cmake/llvm ../llvm_git/polly
 
 To run unittests, however, you need to have the LLVM source directory around.
 Polly will use the llvm-config of the LLVM you're building against
Index: polly/docs/TipsAndTricks.rst
===
--- polly/docs/TipsAndTricks.rst
+++ polly/docs/TipsAndTricks.rst
@@ -47,7 +47,7 @@
 the regression.
 
 LLVM has a single repository that contains all projects. It can be cloned at:
-``_. How to bisect on a
+``_. How 

[PATCH] D59509: Make static constructors + destructors minsize + cold (except for in -O0)

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Looks reasonable to me.




Comment at: clang/test/CodeGen/static-attr.cpp:4
+
+// WITHOUT-NOT: cold minsize noinline
+// WITH: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]]]

lebedev.ri wrote:
> This is fragile, it may have false-negative if they appear in other order.
I think we guarantee it's emitted in the same order, but it's certainly the 
case that nobody will ever notice if this is failing to fail the test, if other 
attributes get emitted in between in the future.

You can use 3 separate WITHOUT-NOT lines, instead, to avoid both of this kind 
of issue.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59509



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

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

The intricate initialization-order workarounds apparently don't work in all 
build modes, so I've updated this code to have constexpr functions and 
initializations in r355278.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57914



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

[PATCH] D58154: Add support for -fpermissive.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ah -- I now understand your concern, and managing user expectations 
appropriately does seem like a potential concern.

I did not look too closely before into what exactly GCC categorized as 
"permerror" (errors that can be disabled with -fpermissive) vs what Clang 
categorized as "ExtWarn,DefaultError" diagnostics. Looking into it a little 
more now, I think there may not actually be substantial overlap between the two 
right now.

Many errors we have warning flags for are unconditional in GCC, and many that 
gcc allows disabling with -fpermissive are unconditional errors in clang.

You gave examples of the latter already, and e.g. here's a bunch of bogus code 
which you can currently build with -Wno-everything in clang, but most of which 
cannot be disabled in gcc (and I believe most if not all of these flags are 
ExtWarn, so would be disabled by this proposed -fpermissive):
https://godbolt.org/z/81NkJQ

Given that, yeah, it may not actually make sense to call this behavior 
-fpermissive.

I also can't really tell what the intent is behind classifying some diagnostics 
in Clang as ExtWarn,DefaultError and others Warning,DefaultError -- or if there 
even is any useful purpose there at all. I note with special-puzzlement the 
existence of both `ext_return_missing_expr` and `warn_return_missing_expr`.

I guess my feeling now is that perhaps we should just eliminate that 
categorization as meaningless, and just make -Wno-error=everything work 
properly (for anyone who wants to not abort the compile on broken code, but for 
whatever reason also loves to see build-spam so doesn't want -Wno-everything).


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: philip.pfaffe.
Herald added subscribers: cfe-commits, jdoerfert, jfb, dexonsmith, steven_wu, 
hiraditya, mehdi_amini.
Herald added projects: clang, LLVM.

Just as as llvm IR supports explicitly specifying numeric value ids
for instructions, and emits them by default in textual output, now do
the same for blocks.

This is a slightly incompatible change in the textual IR format.

Previously, llvm would parse numeric labels as string names. E.g.

  define void @f() {
br label %"55"
  55:
ret void
  }

defined a label *named* "55", even without needing to be quoted, while
the reference required quoting. Now, if you intend a block label which
looks like a value number to be a name, you must quote it in the
definition too (e.g. `"55":`).

Previously, llvm would print nameless blocks only as a comment, and
would omit it if there was no predecessor. This could cause confusion
for readers of the IR, just as unnamed instructions did prior to the
addition of "%5 = " syntax, back in 2008 (PR2480).

Now, it will always print a label for an unnamed block, with the
exception of the entry block. (IMO it may be better to print it for
the entry-block as well. However, that requires updating many more
tests.)

Thus, the following is supported, and is the canonical printing:

  define i32 @f(i32, i32) {
%3 = add i32 %0, %1
br label %4
  
  4:
ret i32 %3
  }

New test cases covering this behavior are added, and other tests
updated as required.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

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

Add some wording to LangRef and clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/docs/LangRef.rst
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 undef, label %4, label %10
 
-;CHECK:   :4
+;CHECK:   4:
 ;CHECK-NEXT:%5 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:invoke void @f1()
 
-; :4:
+4:
   %5 = load i32*, i32** undef, align 8
   invoke void @f1()
   to label %6 unwind label %1
 
-;CHECK:   :6
+;CHECK:   6:
 ;CHECK-NEXT:%7 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:%8 = load i32*, i32** undef, align 8
 
-; :6:
+6:
   %7 = load i32*, i32** undef, align 8
   %8 = load i32*, i32** undef, align 8
   br i1 true, label %9, label %17
 
-; :9:
+9:
   invoke void @f0()
   to label %10 unwind label %1
 
-; :10:
+10:
   invoke void @f2()
   to label %11 unwind label %1
 
-; :11:
+11:
   %12 = invoke signext i32 undef(i32* null, i32 signext undef, i1 zeroext undef)
   to label %13 unwind label %14
 
-; :13:
+13:
   unreachable
 
-; :14:
+14:
   %15 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :16:
+16:
   unreachable
 
-; :17:
+17:
   ret void
 
 ; uselistorder directives
Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ 

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D58548#1407164 , @dexonsmith wrote:

> I like this idea, and I don’t think the textual IR central is too important.  
> A few things:
>
> - Changes to the IR should always be discussed on llvm-dev.  Did this already 
> happen?


I sent a message ("Improving textual IR format for nameless blocks") 
concurrently with sending this review, and will wait a bit to make sure there's 
no objections.

> - Please update LangRef.

Done.

> - Did you write a script for updating the test cases?  If so, you might 
> consider attaching it to the RFC and linking to it from the commit message as 
> a courtesy to downstream maintainers.

Nope, I didn't; there were few enough instances that it didn't seem worth 
scripting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 187994.
jyknight marked 4 inline comments as done.
jyknight added a comment.

Minor tweaks per comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548

Files:
  clang/test/CodeGenCXX/discard-name-values.cpp
  llgo/test/irgen/imports.go
  llvm/docs/LangRef.rst
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLParser.h
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/IR/AsmWriter.cpp
  llvm/test/Analysis/DominanceFrontier/new_pm_test.ll
  llvm/test/Analysis/RegionInfo/cond_loop.ll
  llvm/test/Analysis/RegionInfo/condition_forward_edge.ll
  llvm/test/Analysis/RegionInfo/condition_same_exit.ll
  llvm/test/Analysis/RegionInfo/condition_simple.ll
  llvm/test/Analysis/RegionInfo/infinite_loop.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_2.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_3.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_4.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_a.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_b.ll
  llvm/test/Analysis/RegionInfo/infinite_loop_5_c.ll
  llvm/test/Analysis/RegionInfo/loop_with_condition.ll
  llvm/test/Analysis/RegionInfo/mix_1.ll
  llvm/test/Analysis/RegionInfo/paper.ll
  llvm/test/Assembler/block-labels.ll
  llvm/test/Assembler/invalid-block-label-num.ll
  llvm/test/CodeGen/X86/atomic-pointer.ll
  llvm/test/Instrumentation/AddressSanitizer/asan-masked-load-store.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime-be.ll
  llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-and-lifetime.ll
  llvm/test/Instrumentation/AddressSanitizer/stack_dynamic_alloca.ll
  llvm/test/Instrumentation/MemorySanitizer/check_access_address.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_kernel_basic.ll
  llvm/test/Instrumentation/MemorySanitizer/msan_x86_bts_asm.ll
  llvm/test/Instrumentation/MemorySanitizer/store-origin.ll
  llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
  llvm/test/Transforms/GVNHoist/pr36787.ll
  llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll

Index: llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
===
--- llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
+++ llvm/test/Transforms/LowerSwitch/2014-06-23-PHIlowering.ll
@@ -2,10 +2,10 @@
 
 define i32 @test(i32 %arg) #0 {
 ; CHECK-LABEL: @test
-; CHECK: ; :2
+; CHECK: 2:
 ; CHECK-NEXT:  %res.0 = phi i32 [ 1, %NodeBlock ], [ 2, %1 ]
 ; CHECK-NEXT:  br label %3
-; CHECK: ; :5
+; CHECK: 5:
 ; CHECK-NEXT:   %res.3 = phi i32 [ 0, %NewDefault ], [ %res.2, %4 ]
 ; CHECK-NEXT:   %6 = add nsw i32 %res.3, 1
 ; CHECK-NEXT:   ret i32 %6
@@ -17,23 +17,23 @@
 i32 4, label %4
   ]
 
-; :1
+1:
   br label %2
 
-; :2
+2:
   %res.0 = phi i32 [ 1, %0 ], [ 2, %1 ]
   br label %3
 
-; :3
+3:
   %res.1 = phi i32 [ 0, %0 ], [ %res.0, %2 ]
   %phitmp = add nsw i32 %res.1, 2
   br label %4
 
-; :4
+4:
   %res.2 = phi i32 [ 1, %0 ], [ %phitmp, %3 ]
   br label %5
 
-; :5
+5:
   %res.3 = phi i32 [ 0, %0 ], [ %res.2, %4 ]
   %6 = add nsw i32 %res.3, 1
   ret i32 %6
Index: llvm/test/Transforms/GVNHoist/pr36787.ll
===
--- llvm/test/Transforms/GVNHoist/pr36787.ll
+++ llvm/test/Transforms/GVNHoist/pr36787.ll
@@ -16,58 +16,58 @@
   invoke void @f0()
   to label %3 unwind label %1
 
-; :1:
+1:
   %2 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :3:
+3:
   br i1 undef, label %4, label %10
 
-;CHECK:   :4
+;CHECK:   4:
 ;CHECK-NEXT:%5 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:invoke void @f1()
 
-; :4:
+4:
   %5 = load i32*, i32** undef, align 8
   invoke void @f1()
   to label %6 unwind label %1
 
-;CHECK:   :6
+;CHECK:   6:
 ;CHECK-NEXT:%7 = load i32*, i32** undef, align 8
 ;CHECK-NEXT:%8 = load i32*, i32** undef, align 8
 
-; :6:
+6:
   %7 = load i32*, i32** undef, align 8
   %8 = load i32*, i32** undef, align 8
   br i1 true, label %9, label %17
 
-; :9:
+9:
   invoke void @f0()
   to label %10 unwind label %1
 
-; :10:
+10:
   invoke void @f2()
   to label %11 unwind label %1
 
-; :11:
+11:
   %12 = invoke signext i32 undef(i32* null, i32 signext undef, i1 zeroext undef)
   to label %13 unwind label %14
 
-; :13:
+13:
   unreachable
 
-; :14:
+14:
   %15 = landingpad { i8*, i32 }
   catch i8* bitcast (i8** @g to i8*)
   catch i8* null
   br label %16
 
-; :16:
+16:
   unreachable
 
-; :17:
+17:
   ret void
 
 ; uselistorder directives
Index: llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
===
--- llvm/test/Instrumentation/SanitizerCoverage/stack-depth.ll
+++ 

[PATCH] D58154: Add support for -fpermissive.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

The errors disabled by this feature are default-error warnings -- you can 
already get the same effect by using -Wno-. Why is it bad to 
additionally allow -fpermissive to disable them? (If we have any diagnostics 
which are currently default-on-warnings which should not _ever_ be 
disable-able, then maybe we should just fix those?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D58154: Add support for -fpermissive.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Hm -- in GCC, -fpermissive has nothing at all to do with 
-pedantic/-pedantic-errors, but I suppose it should be fine to do this way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Sorry, forgot to re-ping this in a timely manner. :)

The discussion on mailing list concluded positively, so waiting for an LG here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548



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

[PATCH] D59346: [X86] Add gcc rotate intrinsics to ia32intrin.h

[James Y Knight via Phabricator via cfe-commits]

jyknight accepted this revision.
jyknight added a comment.

Looks good.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59346



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

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356789: IR: Support parsing numeric block ids, and emit them 
in textual output. (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58548?vs=187994=191913#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58548

Files:
  test/CodeGenCXX/discard-name-values.cpp


Index: test/CodeGenCXX/discard-name-values.cpp
===
--- test/CodeGenCXX/discard-name-values.cpp
+++ test/CodeGenCXX/discard-name-values.cpp
@@ -10,7 +10,7 @@
   // CHECK: br i1 %pred, label %if.then, label %if.end
 
   if (pred) {
-// DISCARDVALUE: ; :2:
+// DISCARDVALUE: 2:
 // DISCARDVALUE-NEXT: tail call void @branch()
 // DISCARDVALUE-NEXT: br label %3
 
@@ -20,7 +20,7 @@
 branch();
   }
 
-  // DISCARDVALUE: ; :3:
+  // DISCARDVALUE: 3:
   // DISCARDVALUE-NEXT: ret i1 %0
 
   // CHECK: if.end:


Index: test/CodeGenCXX/discard-name-values.cpp
===
--- test/CodeGenCXX/discard-name-values.cpp
+++ test/CodeGenCXX/discard-name-values.cpp
@@ -10,7 +10,7 @@
   // CHECK: br i1 %pred, label %if.then, label %if.end
 
   if (pred) {
-// DISCARDVALUE: ; :2:
+// DISCARDVALUE: 2:
 // DISCARDVALUE-NEXT: tail call void @branch()
 // DISCARDVALUE-NEXT: br label %3
 
@@ -20,7 +20,7 @@
 branch();
   }
 
-  // DISCARDVALUE: ; :3:
+  // DISCARDVALUE: 3:
   // DISCARDVALUE-NEXT: ret i1 %0
 
   // CHECK: if.end:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: rsmith, arphaman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This case should emit an -Wimplicit-function-declaration warning.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59711

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/warn-strict-prototypes.c


Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 
-verify %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 
-Wno-implicit-function-declaration -verify %s
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // function declaration with unspecified params
@@ -71,3 +71,9 @@
 // rdar://problem/33251668
 void foo13(...) __attribute__((overloadable));
 void foo13(...) __attribute__((overloadable)) {}
+
+// We should not generate a strict-prototype warning for an implicit
+// declaration.  Leave that up to the implicit-function-declaration warning.
+void foo14(void) {
+  foo14_call(); // no-warning
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4995,7 +4995,10 @@
 break;
   case DeclaratorChunk::Function: {
 const DeclaratorChunk::FunctionTypeInfo  = DeclType.Fun;
-if (FTI.NumParams == 0 && !FTI.isVariadic)
+// We supress the warning when there's no LParen location, as this
+// indicates the declaration was an implicit declaration, which gets
+// warned about separately via -Wimplicit-function-declaration.
+if (FTI.NumParams == 0 && !FTI.isVariadic && 
FTI.getLParenLoc().isValid())
   S.Diag(DeclType.Loc, diag::warn_strict_prototypes)
   << IsBlock
   << FixItHint::CreateInsertion(FTI.getRParenLoc(), "void");


Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -verify %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // function declaration with unspecified params
@@ -71,3 +71,9 @@
 // rdar://problem/33251668
 void foo13(...) __attribute__((overloadable));
 void foo13(...) __attribute__((overloadable)) {}
+
+// We should not generate a strict-prototype warning for an implicit
+// declaration.  Leave that up to the implicit-function-declaration warning.
+void foo14(void) {
+  foo14_call(); // no-warning
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -4995,7 +4995,10 @@
 break;
   case DeclaratorChunk::Function: {
 const DeclaratorChunk::FunctionTypeInfo  = DeclType.Fun;
-if (FTI.NumParams == 0 && !FTI.isVariadic)
+// We supress the warning when there's no LParen location, as this
+// indicates the declaration was an implicit declaration, which gets
+// warned about separately via -Wimplicit-function-declaration.
+if (FTI.NumParams == 0 && !FTI.isVariadic && FTI.getLParenLoc().isValid())
   S.Diag(DeclType.Loc, diag::warn_strict_prototypes)
   << IsBlock
   << FixItHint::CreateInsertion(FTI.getRParenLoc(), "void");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57767: [opaque pointer types] Cleanup CGBuilder's Create*GEP.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353629: [opaque pointer types] Cleanup CGBuilders 
Create*GEP. (authored by jyknight, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57767?vs=185335=186134#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57767

Files:
  cfe/trunk/lib/CodeGen/CGAtomic.cpp
  cfe/trunk/lib/CodeGen/CGBlocks.cpp
  cfe/trunk/lib/CodeGen/CGBuilder.h
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CGCall.cpp
  cfe/trunk/lib/CodeGen/CGCleanup.cpp
  cfe/trunk/lib/CodeGen/CGDecl.cpp
  cfe/trunk/lib/CodeGen/CGExpr.cpp
  cfe/trunk/lib/CodeGen/CGExprAgg.cpp
  cfe/trunk/lib/CodeGen/CGExprComplex.cpp
  cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
  cfe/trunk/lib/CodeGen/CGObjC.cpp
  cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
  cfe/trunk/lib/CodeGen/CGObjCMac.cpp
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
  cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/CodeGen/TargetInfo.cpp

Index: cfe/trunk/lib/CodeGen/CGObjC.cpp
===
--- cfe/trunk/lib/CodeGen/CGObjC.cpp
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp
@@ -158,9 +158,8 @@
 if (ALE) {
   // Emit the element and store it to the appropriate array slot.
   const Expr *Rhs = ALE->getElement(i);
-  LValue LV = MakeAddrLValue(
-  Builder.CreateConstArrayGEP(Objects, i, getPointerSize()),
-  ElementType, AlignmentSource::Decl);
+  LValue LV = MakeAddrLValue(Builder.CreateConstArrayGEP(Objects, i),
+ ElementType, AlignmentSource::Decl);
 
   llvm::Value *value = EmitScalarExpr(Rhs);
   EmitStoreThroughLValue(RValue::get(value), LV, true);
@@ -170,17 +169,15 @@
 } else {
   // Emit the key and store it to the appropriate array slot.
   const Expr *Key = DLE->getKeyValueElement(i).Key;
-  LValue KeyLV = MakeAddrLValue(
-  Builder.CreateConstArrayGEP(Keys, i, getPointerSize()),
-  ElementType, AlignmentSource::Decl);
+  LValue KeyLV = MakeAddrLValue(Builder.CreateConstArrayGEP(Keys, i),
+ElementType, AlignmentSource::Decl);
   llvm::Value *keyValue = EmitScalarExpr(Key);
   EmitStoreThroughLValue(RValue::get(keyValue), KeyLV, /*isInit=*/true);
 
   // Emit the value and store it to the appropriate array slot.
   const Expr *Value = DLE->getKeyValueElement(i).Value;
-  LValue ValueLV = MakeAddrLValue(
-  Builder.CreateConstArrayGEP(Objects, i, getPointerSize()),
-  ElementType, AlignmentSource::Decl);
+  LValue ValueLV = MakeAddrLValue(Builder.CreateConstArrayGEP(Objects, i),
+  ElementType, AlignmentSource::Decl);
   llvm::Value *valueValue = EmitScalarExpr(Value);
   EmitStoreThroughLValue(RValue::get(valueValue), ValueLV, /*isInit=*/true);
   if (TrackNeededObjects) {
@@ -1666,8 +1663,8 @@
   // Save the initial mutations value.  This is the value at an
   // address that was written into the state object by
   // countByEnumeratingWithState:objects:count:.
-  Address StateMutationsPtrPtr = Builder.CreateStructGEP(
-  StatePtr, 2, 2 * getPointerSize(), "mutationsptr.ptr");
+  Address StateMutationsPtrPtr =
+  Builder.CreateStructGEP(StatePtr, 2, "mutationsptr.ptr");
   llvm::Value *StateMutationsPtr
 = Builder.CreateLoad(StateMutationsPtrPtr, "mutationsptr");
 
@@ -1748,8 +1745,8 @@
   // Fetch the buffer out of the enumeration state.
   // TODO: this pointer should actually be invariant between
   // refreshes, which would help us do certain loop optimizations.
-  Address StateItemsPtr = Builder.CreateStructGEP(
-  StatePtr, 1, getPointerSize(), "stateitems.ptr");
+  Address StateItemsPtr =
+  Builder.CreateStructGEP(StatePtr, 1, "stateitems.ptr");
   llvm::Value *EnumStateItems =
 Builder.CreateLoad(StateItemsPtr, "stateitems");
 
Index: cfe/trunk/lib/CodeGen/CGExprComplex.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprComplex.cpp
+++ cfe/trunk/lib/CodeGen/CGExprComplex.cpp
@@ -327,15 +327,12 @@
 
 Address CodeGenFunction::emitAddrOfRealComponent(Address addr,
  QualType complexType) {
-  CharUnits offset = CharUnits::Zero();
-  return Builder.CreateStructGEP(addr, 0, offset, addr.getName() + ".realp");
+  return Builder.CreateStructGEP(addr, 0, addr.getName() + ".realp");
 }
 
 Address CodeGenFunction::emitAddrOfImagComponent(Address addr,
  QualType complexType) {
-  QualType eltType = complexType->castAs()->getElementType();
-  CharUnits offset = 

[PATCH] D58091: Customize warnings for missing built-in type

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

I think this warning (-Wbuiltin-requires-header) doesn't really make sense as 
its own warning.

We already have two related (on-by-default) warnings.

For declarations:

  test.c:1:6: warning: incompatible redeclaration of library function 'exit' 
[-Wincompatible-library-redeclaration]
  long exit(char *);
   ^
  test.c:1:6: note: 'exit' is a builtin with type 'void (int) 
__attribute__((noreturn))'

And for uses:

  test2.c:1:13: warning: implicitly declaring library function 'exit' with type 
'void (int) __attribute__((noreturn))' [-Wimplicit-function-declaration]
  int foo() { exit(0); }
  ^
  test2.c:1:13: note: include the header  or explicitly provide a 
declaration for 'exit'

I think for a declaration, if we cannot construct the appropriate type, we 
should be treating all declarations as an incompatible redeclaration, and 
explain why in an attached note, like:

  warning: incompatible redeclaration of library function 'exit' 
[-Wincompatible-library-redeclaration]
  note: missing declaration of type 'jmp_buf' for argument 1 of standard 
function signature.

For a usage, we could emit something like:

  warning: implicit declaration of library function 'setjmp' 
[-Wimplicit-function-declaration]
  note: missing declaration of type 'jmp_buf' for argument 1.
  note: include the header  or explicitly provide a declaration for 
'setjmp'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58091



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

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

[James Y Knight via Phabricator via cfe-commits]

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

Looks reasonable to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58120



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

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353355: [opaque pointer types] Pass through function types 
for TLS (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57801?vs=185467=185677#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57801

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGCXXABI.h
  lib/CodeGen/CGDeclCXX.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp

Index: lib/CodeGen/CGCXX.cpp
===
--- lib/CodeGen/CGCXX.cpp
+++ lib/CodeGen/CGCXX.cpp
@@ -227,10 +227,11 @@
   return Fn;
 }
 
-llvm::Constant *CodeGenModule::getAddrOfCXXStructor(
+llvm::FunctionCallee CodeGenModule::getAddrAndTypeOfCXXStructor(
 const CXXMethodDecl *MD, StructorType Type, const CGFunctionInfo *FnInfo,
 llvm::FunctionType *FnType, bool DontDefer,
 ForDefinition_t IsForDefinition) {
+
   GlobalDecl GD;
   if (auto *CD = dyn_cast(MD)) {
 GD = GlobalDecl(CD, toCXXCtorType(Type));
@@ -249,9 +250,10 @@
 FnType = getTypes().GetFunctionType(*FnInfo);
   }
 
-  return GetOrCreateLLVMFunction(
+  llvm::Constant *Ptr = GetOrCreateLLVMFunction(
   getMangledName(GD), FnType, GD, /*ForVTable=*/false, DontDefer,
   /*isThunk=*/false, /*ExtraAttrs=*/llvm::AttributeList(), IsForDefinition);
+  return {FnType, Ptr};
 }
 
 static CGCallee BuildAppleKextVirtualCall(CodeGenFunction ,
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -394,7 +394,8 @@
llvm::GlobalVariable *DeclPtr,
bool PerformInit) override;
   void registerGlobalDtor(CodeGenFunction , const VarDecl ,
-  llvm::Constant *Dtor, llvm::Constant *Addr) override;
+  llvm::FunctionCallee Dtor,
+  llvm::Constant *Addr) override;
 
   //  Notes on array cookies =
   //
@@ -,7 +2223,7 @@
 }
 
 static void emitGlobalDtorWithTLRegDtor(CodeGenFunction , const VarDecl ,
-llvm::Constant *Dtor,
+llvm::FunctionCallee Dtor,
 llvm::Constant *Addr) {
   // Create a function which calls the destructor.
   llvm::Constant *DtorStub = CGF.createAtExitStub(VD, Dtor, Addr);
@@ -2241,7 +2242,7 @@
 }
 
 void MicrosoftCXXABI::registerGlobalDtor(CodeGenFunction , const VarDecl ,
- llvm::Constant *Dtor,
+ llvm::FunctionCallee Dtor,
  llvm::Constant *Addr) {
   if (D.isNoDestroy(CGM.getContext()))
 return;
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3924,12 +3924,12 @@
   void EmitCXXGlobalVarDeclInit(const VarDecl , llvm::Constant *DeclPtr,
 bool PerformInit);
 
-  llvm::Constant *createAtExitStub(const VarDecl , llvm::Constant *Dtor,
+  llvm::Function *createAtExitStub(const VarDecl , llvm::FunctionCallee Dtor,
llvm::Constant *Addr);
 
   /// Call atexit() with a function that passes the given argument to
   /// the given function.
-  void registerGlobalDtorWithAtExit(const VarDecl , llvm::Constant *fn,
+  void registerGlobalDtorWithAtExit(const VarDecl , llvm::FunctionCallee fn,
 llvm::Constant *addr);
 
   /// Call atexit() with function dtorStub.
@@ -3962,8 +3962,8 @@
   /// variables.
   void GenerateCXXGlobalDtorsFunc(
   llvm::Function *Fn,
-  const std::vector>
-  );
+  const std::vector> );
 
   void GenerateCXXGlobalVarDeclInitFunc(llvm::Function *Fn,
 const VarDecl *D,
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -330,7 +330,7 @@
   switch (M->getStorageDuration()) {
   case SD_Static:
   case SD_Thread: {
-llvm::Constant *CleanupFn;
+llvm::FunctionCallee CleanupFn;
 llvm::Constant *CleanupArg;
 if (E->getType()->isArrayType()) {
   CleanupFn = CodeGenFunction(CGF.CGM).generateDestroyHelper(
@@ -339,8 +339,8 @@
   dyn_cast_or_null(M->getExtendingDecl()));
   CleanupArg = llvm::Constant::getNullValue(CGF.Int8PtrTy);
 } else {
-  CleanupFn = CGF.CGM.getAddrOfCXXStructor(ReferenceTemporaryDtor,
-   StructorType::Complete);
+  CleanupFn = 

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353356: [opaque pointer types] Make EmitCall pass Function 
Types to (authored by jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57804?vs=185476=185678#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57804

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGCall.h

Index: lib/CodeGen/CGCall.h
===
--- lib/CodeGen/CGCall.h
+++ lib/CodeGen/CGCall.h
@@ -204,12 +204,9 @@
   assert(isVirtual());
   return VirtualInfo.Addr;
 }
-
-llvm::FunctionType *getFunctionType() const {
-  if (isVirtual())
-return VirtualInfo.FTy;
-  return cast(
-  getFunctionPointer()->getType()->getPointerElementType());
+llvm::FunctionType *getVirtualFunctionType() const {
+  assert(isVirtual());
+  return VirtualInfo.FTy;
 }
 
 /// If this is a delayed callee computation of some sort, prepare
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -3826,7 +3826,7 @@
   QualType RetTy = CallInfo.getReturnType();
   const ABIArgInfo  = CallInfo.getReturnInfo();
 
-  llvm::FunctionType *IRFuncTy = Callee.getFunctionType();
+  llvm::FunctionType *IRFuncTy = getTypes().GetFunctionType(CallInfo);
 
 #ifndef NDEBUG
   if (!(CallInfo.isVariadic() && CallInfo.getArgStruct())) {
@@ -3837,8 +3837,13 @@
 //
 // In other cases, we assert that the types match up (until pointers stop
 // having pointee types).
-llvm::FunctionType *IRFuncTyFromInfo = getTypes().GetFunctionType(CallInfo);
-assert(IRFuncTy == IRFuncTyFromInfo);
+llvm::Type *TypeFromVal;
+if (Callee.isVirtual())
+  TypeFromVal = Callee.getVirtualFunctionType();
+else
+  TypeFromVal =
+  Callee.getFunctionPointer()->getType()->getPointerElementType();
+assert(IRFuncTy == TypeFromVal);
   }
 #endif
 
@@ -4207,8 +4212,8 @@
   // cases, we can't do any parameter mismatch checks.  Give up and bitcast
   // the callee.
   unsigned CalleeAS = CalleePtr->getType()->getPointerAddressSpace();
-  auto FnTy = getTypes().GetFunctionType(CallInfo)->getPointerTo(CalleeAS);
-  CalleePtr = Builder.CreateBitCast(CalleePtr, FnTy);
+  CalleePtr =
+  Builder.CreateBitCast(CalleePtr, IRFuncTy->getPointerTo(CalleeAS));
 } else {
   llvm::Type *LastParamTy =
   IRFuncTy->getParamType(IRFuncTy->getNumParams() - 1);
@@ -4240,19 +4245,20 @@
   //
   // This makes the IR nicer, but more importantly it ensures that we
   // can inline the function at -O0 if it is marked always_inline.
-  auto simplifyVariadicCallee = [](llvm::Value *Ptr) -> llvm::Value* {
-llvm::FunctionType *CalleeFT =
-  cast(Ptr->getType()->getPointerElementType());
+  auto simplifyVariadicCallee = [](llvm::FunctionType *CalleeFT,
+   llvm::Value *Ptr) -> llvm::Function * {
 if (!CalleeFT->isVarArg())
-  return Ptr;
+  return nullptr;
 
-llvm::ConstantExpr *CE = dyn_cast(Ptr);
-if (!CE || CE->getOpcode() != llvm::Instruction::BitCast)
-  return Ptr;
+// Get underlying value if it's a bitcast
+if (llvm::ConstantExpr *CE = dyn_cast(Ptr)) {
+  if (CE->getOpcode() == llvm::Instruction::BitCast)
+Ptr = CE->getOperand(0);
+}
 
-llvm::Function *OrigFn = dyn_cast(CE->getOperand(0));
+llvm::Function *OrigFn = dyn_cast(Ptr);
 if (!OrigFn)
-  return Ptr;
+  return nullptr;
 
 llvm::FunctionType *OrigFT = OrigFn->getFunctionType();
 
@@ -4261,15 +4267,19 @@
 if (OrigFT->isVarArg() ||
 OrigFT->getNumParams() != CalleeFT->getNumParams() ||
 OrigFT->getReturnType() != CalleeFT->getReturnType())
-  return Ptr;
+  return nullptr;
 
 for (unsigned i = 0, e = OrigFT->getNumParams(); i != e; ++i)
   if (OrigFT->getParamType(i) != CalleeFT->getParamType(i))
-return Ptr;
+return nullptr;
 
 return OrigFn;
   };
-  CalleePtr = simplifyVariadicCallee(CalleePtr);
+
+  if (llvm::Function *OrigFn = simplifyVariadicCallee(IRFuncTy, CalleePtr)) {
+CalleePtr = OrigFn;
+IRFuncTy = OrigFn->getFunctionType();
+  }
 
   // 3. Perform the actual call.
 
@@ -4364,10 +4374,10 @@
   // Emit the actual call/invoke instruction.
   llvm::CallBase *CI;
   if (!InvokeDest) {
-CI = Builder.CreateCall(CalleePtr, IRCallArgs, BundleList);
+CI = Builder.CreateCall(IRFuncTy, CalleePtr, IRCallArgs, BundleList);
   } else {
 llvm::BasicBlock *Cont = createBasicBlock("invoke.cont");
-CI = Builder.CreateInvoke(CalleePtr, Cont, InvokeDest, IRCallArgs,
+CI = Builder.CreateInvoke(IRFuncTy, CalleePtr, Cont, InvokeDest, IRCallArgs,
   BundleList);
 EmitBlock(Cont);
   }

[PATCH] D57794: Fix MSVC constructor call extension after b92d290e48e9 (r353181).

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL353246: Fix MSVC constructor call extension after 
b92d290e48e9 (r353181). (authored by jyknight, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57794?vs=185426=185430#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57794

Files:
  cfe/trunk/lib/CodeGen/CGExprCXX.cpp
  cfe/trunk/test/CodeGenCXX/constructor-direct-call.cpp

Index: cfe/trunk/test/CodeGenCXX/constructor-direct-call.cpp
===
--- cfe/trunk/test/CodeGenCXX/constructor-direct-call.cpp
+++ cfe/trunk/test/CodeGenCXX/constructor-direct-call.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK32 --check-prefix=CHECK
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK64 --check-prefix=CHECK
 
 class Test1 {
 public:
@@ -9,7 +10,8 @@
   Test1 var;
   var.Test1::Test1();
 
-  // CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
+  // CHECK32:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
+  // CHECK64:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 4, i1 false)
   var.Test1::Test1(var);
 }
 
@@ -22,13 +24,16 @@
 
 void f2() {
   // CHECK:  %var = alloca %class.Test2, align 4
-  // CHECK-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK32-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK64-NEXT:  %call = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var)
   Test2 var;
 
-  // CHECK-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK32-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK64-NEXT:  %call1 = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var)
   var.Test2::Test2();
 
-  // CHECK:  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false)
+  // CHECK32:  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false)
+  // CHECK64:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 8, i1 false)
   var.Test2::Test2(var);
 }
 
@@ -45,15 +50,19 @@
 };
 
 void f3() {
-  // CHECK: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK32: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK64: %call = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var)
   Test3 var;
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2)
+  // CHECK64-NEXT: %call1 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var2)
   Test3 var2;
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK64-NEXT: %call2 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var)
   var.Test3::Test3();
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
+  // CHECK64-NEXT: %call3 = call %class.Test3* @"??0Test3@@QEAA@AEBV0@@Z"(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
   var.Test3::Test3(var2);
 }
Index: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp
@@ -249,13 +249,25 @@
 This = EmitLValue(Base);
   }
 
+  if (const CXXConstructorDecl *Ctor = dyn_cast(MD)) {
+// This is the MSVC p->Ctor::Ctor(...) extension. We assume that's
+// constructing a new complete object of type Ctor.
+assert(!RtlArgs);
+assert(ReturnValue.isNull() && "Constructor shouldn't have return value");
+CallArgList Args;
+commonEmitCXXMemberOrOperatorCall(
+*this, Ctor, This.getPointer(), /*ImplicitParam=*/nullptr,
+/*ImplicitParamTy=*/QualType(), CE, Args, nullptr);
+
+EmitCXXConstructorCall(Ctor, Ctor_Complete, /*ForVirtualBase=*/false,
+   /*Delegating=*/false, This.getAddress(), Args,
+   AggValueSlot::DoesNotOverlap, CE->getExprLoc(),
+   /*NewPointerIsChecked=*/false);
+return 

[PATCH] D57794: Fix MSVC constructor call extension after b92d290e48e9 (r353181).

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added reviewers: thakis, rsmith.
Herald added subscribers: cfe-commits, mstorsjo.
Herald added a project: clang.

The assert added to EmitCall there was triggering in Windows Chromium
builds, due to a mismatch of the return type.

The MSVC constructor call extension (`this->Foo::Foo()`) was emitting
the constructor call from 'EmitCXXMemberOrOperatorMemberCallExpr' via
calling 'EmitCXXMemberOrOperatorCall', instead of
'EmitCXXConstructorCall'. On targets where HasThisReturn is true, that
was failing to set the proper return type in the call info.

Switching to calling EmitCXXConstructorCall also allowed removing some
code e.g. the trivial copy/move support, which is already handled in
EmitCXXConstructorCall.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57794

Files:
  clang/lib/CodeGen/CGExprCXX.cpp
  clang/test/CodeGenCXX/constructor-direct-call.cpp

Index: clang/test/CodeGenCXX/constructor-direct-call.cpp
===
--- clang/test/CodeGenCXX/constructor-direct-call.cpp
+++ clang/test/CodeGenCXX/constructor-direct-call.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple i686-pc-mingw32 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK32 --check-prefix=CHECK
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc19.11.0 -fms-extensions -Wmicrosoft %s -emit-llvm -o - | FileCheck %s --check-prefix=CHECK64 --check-prefix=CHECK
 
 class Test1 {
 public:
@@ -9,7 +10,8 @@
   Test1 var;
   var.Test1::Test1();
 
-  // CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
+  // CHECK32:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 4, i1 false)
+  // CHECK64:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 4, i1 false)
   var.Test1::Test1(var);
 }
 
@@ -22,13 +24,16 @@
 
 void f2() {
   // CHECK:  %var = alloca %class.Test2, align 4
-  // CHECK-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK32-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK64-NEXT:  %call = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var)
   Test2 var;
 
-  // CHECK-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK32-NEXT:  call x86_thiscallcc void @_ZN5Test2C1Ev(%class.Test2* %var)
+  // CHECK64-NEXT:  %call1 = call %class.Test2* @"??0Test2@@QEAA@XZ"(%class.Test2* %var)
   var.Test2::Test2();
 
-  // CHECK:  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false)
+  // CHECK32:  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i32 8, i1 false)
+  // CHECK64:  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %{{.*}}, i8* align 4 %{{.*}}, i64 8, i1 false)
   var.Test2::Test2(var);
 }
 
@@ -45,15 +50,19 @@
 };
 
 void f3() {
-  // CHECK: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK32: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK64: %call = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var)
   Test3 var;
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var2)
+  // CHECK64-NEXT: %call1 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var2)
   Test3 var2;
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1Ev(%class.Test3* %var)
+  // CHECK64-NEXT: %call2 = call %class.Test3* @"??0Test3@@QEAA@XZ"(%class.Test3* %var)
   var.Test3::Test3();
 
-  // CHECK-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
+  // CHECK32-NEXT: call x86_thiscallcc void @_ZN5Test3C1ERKS_(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
+  // CHECK64-NEXT: %call3 = call %class.Test3* @"??0Test3@@QEAA@AEBV0@@Z"(%class.Test3* %var, %class.Test3* dereferenceable({{[0-9]+}}) %var2)
   var.Test3::Test3(var2);
 }
Index: clang/lib/CodeGen/CGExprCXX.cpp
===
--- clang/lib/CodeGen/CGExprCXX.cpp
+++ clang/lib/CodeGen/CGExprCXX.cpp
@@ -249,13 +249,25 @@
 This = EmitLValue(Base);
   }
 
+  if (const CXXConstructorDecl *Ctor = dyn_cast(MD)) {
+// This is the MSVC p->Ctor::Ctor(...) extension. We assume that's
+// constructing a new complete object of type Ctor.
+assert(!RtlArgs);
+assert(ReturnValue.isNull() && "Constructor shouldn't have return value");
+CallArgList Args;
+commonEmitCXXMemberOrOperatorCall(
+*this, Ctor, This.getPointer(), /*ImplicitParam=*/nullptr,
+/*ImplicitParamTy=*/QualType(), CE, 

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

[James Y Knight via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57801

Files:
  clang/lib/CodeGen/CGCXX.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/MicrosoftCXXABI.cpp

Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -394,7 +394,8 @@
llvm::GlobalVariable *DeclPtr,
bool PerformInit) override;
   void registerGlobalDtor(CodeGenFunction , const VarDecl ,
-  llvm::Constant *Dtor, llvm::Constant *Addr) override;
+  llvm::FunctionCallee Dtor,
+  llvm::Constant *Addr) override;
 
   //  Notes on array cookies =
   //
@@ -,7 +2223,7 @@
 }
 
 static void emitGlobalDtorWithTLRegDtor(CodeGenFunction , const VarDecl ,
-llvm::Constant *Dtor,
+llvm::FunctionCallee Dtor,
 llvm::Constant *Addr) {
   // Create a function which calls the destructor.
   llvm::Constant *DtorStub = CGF.createAtExitStub(VD, Dtor, Addr);
@@ -2241,7 +2242,7 @@
 }
 
 void MicrosoftCXXABI::registerGlobalDtor(CodeGenFunction , const VarDecl ,
- llvm::Constant *Dtor,
+ llvm::FunctionCallee Dtor,
  llvm::Constant *Addr) {
   if (D.isNoDestroy(CGM.getContext()))
 return;
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -328,7 +328,8 @@
llvm::GlobalVariable *DeclPtr,
bool PerformInit) override;
   void registerGlobalDtor(CodeGenFunction , const VarDecl ,
-  llvm::Constant *dtor, llvm::Constant *addr) override;
+  llvm::FunctionCallee dtor,
+  llvm::Constant *addr) override;
 
   llvm::Function *getOrCreateThreadLocalWrapper(const VarDecl *VD,
 llvm::Value *Val);
@@ -2284,9 +2285,8 @@
 
 /// Register a global destructor using __cxa_atexit.
 static void emitGlobalDtorWithCXAAtExit(CodeGenFunction ,
-llvm::Constant *dtor,
-llvm::Constant *addr,
-bool TLS) {
+llvm::FunctionCallee dtor,
+llvm::Constant *addr, bool TLS) {
   const char *Name = "__cxa_atexit";
   if (TLS) {
 const llvm::Triple  = CGF.getTarget().getTriple();
@@ -2322,11 +2322,10 @@
 // function.
 addr = llvm::Constant::getNullValue(CGF.Int8PtrTy);
 
-  llvm::Value *args[] = {
-llvm::ConstantExpr::getBitCast(dtor, dtorTy),
-llvm::ConstantExpr::getBitCast(addr, CGF.Int8PtrTy),
-handle
-  };
+  llvm::Value *args[] = {llvm::ConstantExpr::getBitCast(
+ cast(dtor.getCallee()), dtorTy),
+ llvm::ConstantExpr::getBitCast(addr, CGF.Int8PtrTy),
+ handle};
   CGF.EmitNounwindRuntimeCall(atexit, args);
 }
 
@@ -2375,9 +2374,8 @@
 }
 
 /// Register a global destructor as best as we know how.
-void ItaniumCXXABI::registerGlobalDtor(CodeGenFunction ,
-   const VarDecl ,
-   llvm::Constant *dtor,
+void ItaniumCXXABI::registerGlobalDtor(CodeGenFunction , const VarDecl ,
+   llvm::FunctionCallee dtor,
llvm::Constant *addr) {
   if (D.isNoDestroy(CGM.getContext()))
 return;
@@ -2541,6 +2539,8 @@
   getMangleContext().mangleItaniumThreadLocalInit(VD, Out);
 }
 
+llvm::FunctionType *InitFnTy = llvm::FunctionType::get(CGM.VoidTy, false);
+
 // If we have a definition for the variable, emit the initialization
 // function as an alias to the global Init function (if any). Otherwise,
 // produce a declaration of the initialization function.
@@ -2559,8 +2559,7 @@
   // This function will not exist if the TU defining the thread_local
   // variable in question does not need any dynamic initialization for
   // its thread_local variables.
-  llvm::FunctionType *FnTy = llvm::FunctionType::get(CGM.VoidTy, false);
-  Init = llvm::Function::Create(FnTy,
+  Init = 

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

[James Y Knight via Phabricator via cfe-commits]

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

Also, remove the getFunctionType() function from CGCallee, since it
accesses the pointee type of the value. The only use was in EmitCall,
so just inline it into the debug assertion.

This is the last of the changes for Call and Invoke in clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57804

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGCall.h

Index: clang/lib/CodeGen/CGCall.h
===
--- clang/lib/CodeGen/CGCall.h
+++ clang/lib/CodeGen/CGCall.h
@@ -204,12 +204,9 @@
   assert(isVirtual());
   return VirtualInfo.Addr;
 }
-
-llvm::FunctionType *getFunctionType() const {
-  if (isVirtual())
-return VirtualInfo.FTy;
-  return cast(
-  getFunctionPointer()->getType()->getPointerElementType());
+llvm::FunctionType *getVirtualFunctionType() const {
+  assert(isVirtual());
+  return VirtualInfo.FTy;
 }
 
 /// If this is a delayed callee computation of some sort, prepare
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -3826,7 +3826,7 @@
   QualType RetTy = CallInfo.getReturnType();
   const ABIArgInfo  = CallInfo.getReturnInfo();
 
-  llvm::FunctionType *IRFuncTy = Callee.getFunctionType();
+  llvm::FunctionType *IRFuncTy = getTypes().GetFunctionType(CallInfo);
 
 #ifndef NDEBUG
   if (!(CallInfo.isVariadic() && CallInfo.getArgStruct())) {
@@ -3837,8 +3837,13 @@
 //
 // In other cases, we assert that the types match up (until pointers stop
 // having pointee types).
-llvm::FunctionType *IRFuncTyFromInfo = getTypes().GetFunctionType(CallInfo);
-assert(IRFuncTy == IRFuncTyFromInfo);
+llvm::Type *TypeFromVal;
+if (Callee.isVirtual())
+  TypeFromVal = Callee.getVirtualFunctionType();
+else
+  TypeFromVal =
+  Callee.getFunctionPointer()->getType()->getPointerElementType();
+assert(IRFuncTy == TypeFromVal);
   }
 #endif
 
@@ -4207,8 +4212,8 @@
   // cases, we can't do any parameter mismatch checks.  Give up and bitcast
   // the callee.
   unsigned CalleeAS = CalleePtr->getType()->getPointerAddressSpace();
-  auto FnTy = getTypes().GetFunctionType(CallInfo)->getPointerTo(CalleeAS);
-  CalleePtr = Builder.CreateBitCast(CalleePtr, FnTy);
+  CalleePtr =
+  Builder.CreateBitCast(CalleePtr, IRFuncTy->getPointerTo(CalleeAS));
 } else {
   llvm::Type *LastParamTy =
   IRFuncTy->getParamType(IRFuncTy->getNumParams() - 1);
@@ -4240,19 +4245,20 @@
   //
   // This makes the IR nicer, but more importantly it ensures that we
   // can inline the function at -O0 if it is marked always_inline.
-  auto simplifyVariadicCallee = [](llvm::Value *Ptr) -> llvm::Value* {
-llvm::FunctionType *CalleeFT =
-  cast(Ptr->getType()->getPointerElementType());
+  auto simplifyVariadicCallee = [](llvm::FunctionType *CalleeFT,
+   llvm::Value *Ptr) -> llvm::Function * {
 if (!CalleeFT->isVarArg())
-  return Ptr;
+  return nullptr;
 
-llvm::ConstantExpr *CE = dyn_cast(Ptr);
-if (!CE || CE->getOpcode() != llvm::Instruction::BitCast)
-  return Ptr;
+// Get underlying value if it's a bitcast
+if (llvm::ConstantExpr *CE = dyn_cast(Ptr)) {
+  if (CE->getOpcode() == llvm::Instruction::BitCast)
+Ptr = CE->getOperand(0);
+}
 
-llvm::Function *OrigFn = dyn_cast(CE->getOperand(0));
+llvm::Function *OrigFn = dyn_cast(Ptr);
 if (!OrigFn)
-  return Ptr;
+  return nullptr;
 
 llvm::FunctionType *OrigFT = OrigFn->getFunctionType();
 
@@ -4261,15 +4267,19 @@
 if (OrigFT->isVarArg() ||
 OrigFT->getNumParams() != CalleeFT->getNumParams() ||
 OrigFT->getReturnType() != CalleeFT->getReturnType())
-  return Ptr;
+  return nullptr;
 
 for (unsigned i = 0, e = OrigFT->getNumParams(); i != e; ++i)
   if (OrigFT->getParamType(i) != CalleeFT->getParamType(i))
-return Ptr;
+return nullptr;
 
 return OrigFn;
   };
-  CalleePtr = simplifyVariadicCallee(CalleePtr);
+
+  if (llvm::Function *OrigFn = simplifyVariadicCallee(IRFuncTy, CalleePtr)) {
+CalleePtr = OrigFn;
+IRFuncTy = OrigFn->getFunctionType();
+  }
 
   // 3. Perform the actual call.
 
@@ -4364,10 +4374,10 @@
   // Emit the actual call/invoke instruction.
   llvm::CallBase *CI;
   if (!InvokeDest) {
-CI = Builder.CreateCall(CalleePtr, IRCallArgs, BundleList);
+CI = Builder.CreateCall(IRFuncTy, CalleePtr, IRCallArgs, BundleList);
   } else {
 llvm::BasicBlock *Cont = createBasicBlock("invoke.cont");
-CI = Builder.CreateInvoke(CalleePtr, Cont, InvokeDest, IRCallArgs,
+CI = Builder.CreateInvoke(IRFuncTy, CalleePtr, 

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lib/Basic/Targets/RISCV.h:94
+if (HasA)
+  MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32;
+  }

MaxAtomicPromoteWidth is an incompatible ABI-change kind of thing.

We should set that to the maximum atomic size this target ABI will _EVER_ 
support with any hardware, since it changes the data layout for atomic types.

MaxAtomicInlineWidth can change based on hardware, and be increased in the 
future if other hardware is introduced, but MaxAtomicPromoteWidth shouldn't.

I think it should be set it to 64 for most 32-bit platforms, and 128 for most 
64-bit platforms.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57450



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

[PATCH] D57330: Adjust documentation for git migration.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
jyknight marked 2 inline comments as done.
Closed by commit rC352514: Adjust documentation for git migration. (authored by 
jyknight, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57330?vs=183891=184098#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57330

Files:
  .gitignore
  docs/ClangPlugins.rst
  docs/ClangTools.rst
  docs/ControlFlowIntegrityDesign.rst
  docs/InternalsManual.rst
  docs/LibASTMatchersTutorial.rst
  docs/LibTooling.rst
  docs/Toolchain.rst
  lib/CodeGen/CGOpenMPRuntime.cpp
  www/analyzer/checker_dev_manual.html
  www/get_started.html
  www/hacking.html
  www/menu.html.incl

Index: docs/LibTooling.rst
===
--- docs/LibTooling.rst
+++ docs/LibTooling.rst
@@ -196,6 +196,6 @@
 Linking
 ^^^
 
-For a list of libraries to link, look at one of the tools' Makefiles (for
-example `clang-check/Makefile
-`_).
+For a list of libraries to link, look at one of the tools' CMake files (for
+example `clang-check/CMakeList.txt
+`_).
Index: docs/InternalsManual.rst
===
--- docs/InternalsManual.rst
+++ docs/InternalsManual.rst
@@ -1686,7 +1686,7 @@
 ^^^
 The first step to adding a new attribute to Clang is to add its definition to
 `include/clang/Basic/Attr.td
-`_.
+`_.
 This tablegen definition must derive from the ``Attr`` (tablegen, not
 semantic) type, or one of its derivatives. Most attributes will derive from the
 ``InheritableAttr`` type, which specifies that the attribute can be inherited by
@@ -1748,10 +1748,10 @@
 either ``diag::warn_attribute_wrong_decl_type`` or
 ``diag::err_attribute_wrong_decl_type``, and the parameter enumeration is found
 in `include/clang/Sema/ParsedAttr.h
-`_
+`_
 If a previously unused Decl node is added to the ``SubjectList``, the logic used
 to automatically determine the diagnostic parameter in `utils/TableGen/ClangAttrEmitter.cpp
-`_
+`_
 may need to be updated.
 
 By default, all subjects in the SubjectList must either be a Decl node defined
@@ -1773,7 +1773,7 @@
 Documentation is table generated on the public web server by a server-side
 process that runs daily. Generally, the documentation for an attribute is a
 stand-alone definition in `include/clang/Basic/AttrDocs.td 
-`_
+`_
 that is named after the attribute being documented.
 
 If the attribute is not for public consumption, or is an implicitly-created
@@ -1824,7 +1824,7 @@
 optional. The associated C++ type of the argument is determined by the argument
 definition type. If the existing argument types are insufficient, new types can
 be created, but it requires modifying `utils/TableGen/ClangAttrEmitter.cpp
-`_
+`_
 to properly support the type.
 
 Other Properties
@@ -1836,7 +1836,7 @@
 If the parsed form of the attribute is more complex, or differs from the
 semantic form, the ``HasCustomParsing`` bit can be set to ``1`` for the class,
 and the parsing code in `Parser::ParseGNUAttributeArgs()
-`_
+`_
 can be updated for the special case. Note that this only applies to arguments
 with a GNU spelling -- attributes with a __declspec spelling currently ignore
 this flag and are handled by ``Parser::ParseMicrosoftDeclSpec``.
@@ -1899,7 +1899,7 @@
 Boilerplate
 ^^^
 All semantic processing of declaration attributes happens in `lib/Sema/SemaDeclAttr.cpp
-`_,
+`_,
 and generally starts in the 

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352535: Fix the behavior of clangs -w flag. (authored 
by jyknight, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53199?vs=183966=184142#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D53199

Files:
  cfe/trunk/lib/Basic/DiagnosticIDs.cpp
  cfe/trunk/test/Frontend/optimization-remark.c
  cfe/trunk/test/Frontend/warning-mapping-2.c
  cfe/trunk/test/Frontend/warning-mapping-4.c
  cfe/trunk/test/Frontend/warning-mapping-5.c
  cfe/trunk/test/Frontend/warning-mapping-6.c
  cfe/trunk/test/Modules/implementation-of-module.m

Index: cfe/trunk/test/Frontend/optimization-remark.c
===
--- cfe/trunk/test/Frontend/optimization-remark.c
+++ cfe/trunk/test/Frontend/optimization-remark.c
@@ -13,6 +13,9 @@
 // RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
 // RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
 //
+// Check that -w doesn't disable remarks.
+// RUN: %clang_cc1 %s -Rpass=inline -w -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+//
 // FIXME: -Reverything should imply -Rpass=.*.
 // RUN: %clang_cc1 %s -Reverything -emit-llvm -o - 2>/dev/null | FileCheck %s --check-prefix=CHECK-NO-REMARKS
 //
Index: cfe/trunk/test/Frontend/warning-mapping-5.c
===
--- cfe/trunk/test/Frontend/warning-mapping-5.c
+++ cfe/trunk/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: cfe/trunk/test/Frontend/warning-mapping-6.c
===
--- cfe/trunk/test/Frontend/warning-mapping-6.c
+++ cfe/trunk/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: cfe/trunk/test/Frontend/warning-mapping-4.c
===
--- cfe/trunk/test/Frontend/warning-mapping-4.c
+++ cfe/trunk/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 // RUN: %clang_cc1 -verify -Wno-error=sign-compare %s
 // RUN: %clang_cc1 -verify -Wsign-compare -w -Wno-error=sign-compare %s
+// RUN: %clang_cc1 -verify -w -Werror=sign-compare %s
 // expected-no-diagnostics
 
 int f0(int x, unsigned y) {
Index: cfe/trunk/test/Frontend/warning-mapping-2.c
===
--- cfe/trunk/test/Frontend/warning-mapping-2.c
+++ cfe/trunk/test/Frontend/warning-mapping-2.c
@@ -1,5 +1,7 @@
-// Check that -w has lower priority than -pedantic-errors.
+// Check that -w takes precedence over -pedantic-errors.
 // RUN: %clang_cc1 -verify -pedantic-errors -w %s
 
-void f0() { f1(); } // expected-error {{implicit declaration of function}}
+// Expect *not* to see a diagnostic for "implicit declaration of function"
+// expected-no-diagnostics
 
+void f0() { f1(); }
Index: cfe/trunk/test/Modules/implementation-of-module.m
===
--- cfe/trunk/test/Modules/implementation-of-module.m
+++ cfe/trunk/test/Modules/implementation-of-module.m
@@ -1,17 +1,17 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -fsyntax-only
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -dM -E -o - 2>&1 | FileCheck %s
 // CHECK-NOT: __building_module
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_left -verify
 
-// RUN: %clang_cc1 -x 

[PATCH] D57330: Adjust documentation for git migration.

[James Y Knight via Phabricator via cfe-commits]

jyknight marked 8 inline comments as done.
jyknight added a comment.

In D57330#1375096 , @labath wrote:

> I am not sure we should be recommending to people to place the build folder 
> under the llvm-project checkout. Is that how people use the monorepo build 
> nowadays? This is not an full out-of-source build, since the build folder is 
> still a subfolder of the repo root (and without a .gitignore file containing 
> the right build folder name, git will complain about untracked files in the 
> repository)...


Well, it's certainly how I do it. I find it the least confusing, because then I 
don't get mixed up as to which source tree a given build directory belongs to 
(or directories, as I may also have build-release, build-debug, etc).




Comment at: libcxx/docs/BuildingLibcxx.rst:57
   $ cd where-you-want-libcxx-to-live
-  $ # Check out llvm, libc++ and libc++abi.
-  $ ``svn co http://llvm.org/svn/llvm-project/llvm/trunk llvm``
-  $ ``svn co http://llvm.org/svn/llvm-project/libcxx/trunk libcxx``
-  $ ``svn co http://llvm.org/svn/llvm-project/libcxxabi/trunk libcxxabi``
+  $ # Check out the sources (includes everything, but we'll only use libcxx)
+  $ ``git clone https://github.com/llvm/llvm-project.git``

mehdi_amini wrote:
> Wonder if it is worth mentioning somewhere how to sparse-checkout?
Yes, I think it likely is worth mentioning that somewhere in the future -- but 
I'd rather not recommend it yet. There's two things that will impact that:

1. We'll need to decide where we plan to keep shared infrastructure (e.g. cmake 
macros etc) used by multiple subprojects within the repository.
2. It seems as though git is actually gaining new support for this kind of 
thing now, might be worth letting that mature a little bit.



Comment at: libcxxabi/www/index.html:86
   mkdir build  cd build
-  cmake .. # on linux you may need to prefix with CC=clang 
CXX=clang++
+  cmake -DLLVM_ENABLE_PROJECTS=libcxxabi ../llvm # on linux you may 
need to prefix with CC=clang CXX=clang++
   make

labath wrote:
> mehdi_amini wrote:
> > Do you now if prefixing with CC is equivalent to -DCMAKE_C_COMPILER?
> It usually is, but it can differ once you start using cache and toolchain 
> files. In any case, using -DCMAKE_C(XX)_COMPILER is the preferred way to do 
> things in cmake.
Changed to specify the cmake arguments.



Comment at: lld/docs/getting_started.rst:71
+ $ cd llvm-project/build (out of source build required)
+ $ cmake -G "Visual Studio 11" ../llvm
 

mehdi_amini wrote:
> Missing -DLLVM_ENABLE_PROJECTS=lld here I believe
Yep fixed.



Comment at: 
lldb/packages/Python/lldbsuite/test/functionalities/command_source/TestCommandSource.py:4
-
-See also http://llvm.org/viewvc/llvm-project?view=rev=109673.
 """

smeenai wrote:
> Would you want to link to the corresponding GitHub commit?
After viewing said commit, I felt it wasn't actually useful to visit to 
understand this file, which is why I removed the link.



Comment at: 
lldb/packages/Python/lldbsuite/test/lang/objc/objc-optimized/TestObjcOptimized.py:4
 
-http://llvm.org/viewvc/llvm-project?rev=126973=rev
 Fixed a bug in the expression parser where the 'this'

smeenai wrote:
> Same here.
Felt the same in this case.


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

https://reviews.llvm.org/D57330



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

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/lib/Basic/DiagnosticIDs.cpp:460-463
+  // Honor -w: this disables all messages mapped to Warning severity, and 
*also*
+  // any diagnostics which are not Error/Fatal by default (that is, they were
+  // upgraded by any of the mechanisms available: -Werror, -pedantic, or 
#pragma
+  // diagnostic)

rsmith wrote:
> I think this would be clearer if phrased the other way around:
> 
> > [...] disables all messages that are not Error/Fatal by default, and also 
> > any diagnostics that are Error/Fatal by default but that have been 
> > downgraded to Warning severity by any of the mechanisms available: 
> > -Wno-error or #pragma diagnostic
Reworded.



Comment at: clang/lib/Basic/DiagnosticIDs.cpp:466
+if (Result == diag::Severity::Warning ||
+!isDefaultMappingAsError((diag::kind)DiagID))
+  return diag::Severity::Ignored;

rsmith wrote:
> I think this change will also cause `-w` to disable all remarks. Was that 
> your intent?
No, that seems like a bug.

Remarks have their own completely-separate set of command-line options; I don't 
think -R should interact with -w. I've added a conditional here, and a test 
case ensuring that.


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

https://reviews.llvm.org/D53199



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

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight updated this revision to Diff 183966.
jyknight marked 2 inline comments as done.
jyknight added a comment.

Fix to not disable remarks, reword comment, adjust implementation-of-module.m 
test-case.


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

https://reviews.llvm.org/D53199

Files:
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/test/Frontend/optimization-remark.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Frontend/warning-mapping-4.c
  clang/test/Frontend/warning-mapping-5.c
  clang/test/Frontend/warning-mapping-6.c
  clang/test/Modules/implementation-of-module.m

Index: clang/test/Modules/implementation-of-module.m
===
--- clang/test/Modules/implementation-of-module.m
+++ clang/test/Modules/implementation-of-module.m
@@ -1,17 +1,17 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -fsyntax-only
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -dM -E -o - 2>&1 | FileCheck %s
 // CHECK-NOT: __building_module
 
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_left -verify
 
-// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -fmodule-implementation-of category_right -emit-pch -o %t.pch
-// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -w -Werror=auto-import %s -I %S/Inputs \
+// RUN: %clang_cc1 -x objective-c-header -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Werror=auto-import %s -I %S/Inputs \
 // RUN: -DWITH_PREFIX -fmodules-ignore-macro=WITH_PREFIX -include-pch %t.pch -fmodule-implementation-of category_right
 
 #ifndef WITH_PREFIX
Index: clang/test/Frontend/warning-mapping-6.c
===
--- /dev/null
+++ clang/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: clang/test/Frontend/warning-mapping-5.c
===
--- clang/test/Frontend/warning-mapping-5.c
+++ clang/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: clang/test/Frontend/warning-mapping-4.c
===
--- clang/test/Frontend/warning-mapping-4.c
+++ clang/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 // RUN: %clang_cc1 -verify -Wno-error=sign-compare %s
 // RUN: %clang_cc1 -verify -Wsign-compare -w -Wno-error=sign-compare %s
+// RUN: %clang_cc1 -verify -w -Werror=sign-compare %s
 // expected-no-diagnostics
 
 int f0(int x, unsigned y) {
Index: clang/test/Frontend/warning-mapping-2.c
===
--- clang/test/Frontend/warning-mapping-2.c
+++ clang/test/Frontend/warning-mapping-2.c
@@ -1,5 +1,7 @@
-// Check that -w has lower priority than -pedantic-errors.
+// Check that -w takes precedence over -pedantic-errors.
 // RUN: %clang_cc1 -verify -pedantic-errors -w %s
 
-void f0() { f1(); } // expected-error {{implicit declaration of function}}
+// Expect *not* to see a diagnostic for "implicit declaration of function"
+// expected-no-diagnostics
 
+void f0() { f1(); }
Index: clang/test/Frontend/optimization-remark.c
===
--- clang/test/Frontend/optimization-remark.c
+++ clang/test/Frontend/optimization-remark.c
@@ -13,6 +13,9 @@
 // RUN: %clang_cc1 %s -Rpass=inline -Rno-everything -emit-llvm 

[PATCH] D57315: [opaque pointer types] Add a FunctionCallee wrapper type, and use it.

[James Y Knight via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC352791: [opaque pointer types] Add a FunctionCallee wrapper 
type, and use it. (authored by jyknight, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57315?vs=183795=184576#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57315

Files:
  lib/CodeGen/CGExpr.cpp


Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3056,7 +3056,7 @@
   bool WithDiag = !CGM.getCodeGenOpts().SanitizeTrap.has(Kind);
 
   llvm::CallInst *CheckCall;
-  llvm::Constant *SlowPathFn;
+  llvm::FunctionCallee SlowPathFn;
   if (WithDiag) {
 llvm::Constant *Info = llvm::ConstantStruct::getAnon(StaticArgs);
 auto *InfoPtr =
@@ -3078,7 +3078,8 @@
 CheckCall = Builder.CreateCall(SlowPathFn, {TypeId, Ptr});
   }
 
-  CGM.setDSOLocal(cast(SlowPathFn->stripPointerCasts()));
+  CGM.setDSOLocal(
+  cast(SlowPathFn.getCallee()->stripPointerCasts()));
   CheckCall->setDoesNotThrow();
 
   EmitBlock(Cont);


Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3056,7 +3056,7 @@
   bool WithDiag = !CGM.getCodeGenOpts().SanitizeTrap.has(Kind);
 
   llvm::CallInst *CheckCall;
-  llvm::Constant *SlowPathFn;
+  llvm::FunctionCallee SlowPathFn;
   if (WithDiag) {
 llvm::Constant *Info = llvm::ConstantStruct::getAnon(StaticArgs);
 auto *InfoPtr =
@@ -3078,7 +3078,8 @@
 CheckCall = Builder.CreateCall(SlowPathFn, {TypeId, Ptr});
   }
 
-  CGM.setDSOLocal(cast(SlowPathFn->stripPointerCasts()));
+  CGM.setDSOLocal(
+  cast(SlowPathFn.getCallee()->stripPointerCasts()));
   CheckCall->setDoesNotThrow();
 
   EmitBlock(Cont);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits