from:"Blower, Melanie via cfe\-commits"

RE: r303798 - For Microsoft compatibility, set fno_operator_names

2017-05-24 Thread Blower, Melanie via cfe-commits

Thanks for the feedback, working on it…

From: Keane, Erich
Sent: Wednesday, May 24, 2017 3:47 PM
To: Nico Weber ; Blower, Melanie 
Cc: cfe-commits ; rnk 
Subject: RE: r303798 - For Microsoft compatibility, set fno_operator_names

Adding Melanie, the author of the patch.

From: tha...@google.com [mailto:tha...@google.com] On 
Behalf Of Nico Weber
Sent: Wednesday, May 24, 2017 12:43 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits 
mailto:cfe-commits@lists.llvm.org>>; rnk 
mailto:r...@chromium.org>>
Subject: Re: r303798 - For Microsoft compatibility, set fno_operator_names

Reviewed here: https://reviews.llvm.org/D33505

Still, please make this warn.

On Wed, May 24, 2017 at 3:42 PM, Nico Weber 
mailto:tha...@google.com>> wrote:
Was this reviewed somewhere?

Please make it so that this emits a warning. We want clang-cl to warn on 
invalid code (and in system headers warnings are suppressed).

On Wed, May 24, 2017 at 3:31 PM, Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Wed May 24 14:31:19 2017
New Revision: 303798

URL: http://llvm.org/viewvc/llvm-project?rev=303798&view=rev
Log:
For Microsoft compatibility, set fno_operator_names

There's a Microsoft header in the Windows SDK which won't
compile with clang because it uses an operator name (and)
as a field name. This patch allows that file to compile by
setting the option which disables operator names.
The header which doesn't compile  C:/Program Files (x86)/
Windows Kits/10/include/10.0.14393.0/um\Query.h:259:40:
error: expected member name or ';' after declaration specifiers

  /* [case()] */ NODERESTRICTION or;
   ~~~ ^

   1 error generated.

Contributed for Melanie Blower

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

Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Parser/MicrosoftExtensions.cpp
cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=303798&r1=303797&r2=303798&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Wed May 24 14:31:19 2017
@@ -1882,7 +1882,7 @@ static void ParseLangArgs(LangOptions &O
   Opts.GNUKeywords = Args.hasFlag(OPT_fgnu_keywords, OPT_fno_gnu_keywords,
   Opts.GNUKeywords);

-  if (Args.hasArg(OPT_fno_operator_names))
+  if (Args.hasArg(OPT_fno_operator_names) || 
Args.hasArg(OPT_fms_compatibility))
 Opts.CXXOperatorNames = 0;

   if (Args.hasArg(OPT_fcuda_is_device))

Modified: cfe/trunk/test/Parser/MicrosoftExtensions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.cpp?rev=303798&r1=303797&r2=303798&view=diff
==
--- cfe/trunk/test/Parser/MicrosoftExtensions.cpp (original)
+++ cfe/trunk/test/Parser/MicrosoftExtensions.cpp Wed May 24 14:31:19 2017
@@ -261,9 +261,8 @@ int __identifier(else} = __identifier(fo
 #define identifier_weird(x) __identifier(x
 int k = identifier_weird(if)); // expected-error {{use of undeclared 
identifier 'if'}}

-// This is a bit weird, but the alternative tokens aren't keywords, and this
-// behavior matches MSVC. FIXME: Consider supporting this anyway.
-extern int __identifier(and) r; // expected-error {{cannot convert '&&' token 
to an identifier}}
+// 'and' is not an operator name with Microsoft compatibility.
+extern int __identifier(and) r; // expected-error {{expected ';' after top 
level declarator}}

 void f() {
   __identifier(() // expected-error {{cannot convert '(' token to an 
identifier}}
@@ -355,7 +354,6 @@ void TestProperty() {
   ++sp.V11;
 }

-//expected-warning@+1 {{C++ operator 'and' (aka '&&') used as a macro name}}
 #define and foo

 struct __declspec(uuid("---C000-0046")) 
__declspec(novtable) IUnknown {};

Modified: cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp?rev=303798&r1=303797&r2=303798&view=diff
==
--- cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp (original)
+++ cfe/trunk/test/Preprocessor/cxx_oper_keyword.cpp Wed May 24 14:31:19 2017
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 %s -E -verify -DOPERATOR_NAMES
 // RUN: %clang_cc1 %s -E -verify -fno-operator-names
+// RUN: %clang_cc1 %s-verify -DTESTWIN -fms-compatibility

 #ifndef OPERATOR_NAMES
 //expected-error@+3 {{token is not a valid binary operator in a preprocessor 
subexpression}}
@@ -29,3 +30,14 @@
 #ifdef and
 #warning and is defined
 #endif
+
+#ifdef TESTWIN
+// For cl compatibility, fno-operator-

RE: r303798 - For Microsoft compatibility, set fno_operator_names

2017-05-25 Thread Blower, Melanie via cfe-commits

Sometimes it’s possible to reason with the open source community, then they 
change their code and make modifications which accommodate the various 
compilers. E.g. we could ask chromium team if they could use the operator 
symbol instead of ‘and’.

From: Nico Weber [mailto:tha...@google.com]
Sent: Thursday, May 25, 2017 12:56 PM
To: Keane, Erich 
Cc: Blower, Melanie ; rnk ; 
cfe-commits ; Hans Wennborg 
Subject: Re: r303798 - For Microsoft compatibility, set fno_operator_names

On Thu, May 25, 2017 at 12:18 PM, Keane, Erich 
mailto:erich.ke...@intel.com>> wrote:
How does chromium compiler in CL?  It seems that it would have a similar 
problem here…

That's a good question! It looks like iso646.h is included, and in MSVC that 
contains something like `#define and &&`. But clang's lib/Headers/iso646.h 
assumes that the compiler provides this and doesn't define `and`. So for the 
reland, that header would have to check if the operator name is disabled, and 
if so, define it. (Or, better, maybe we can come up with something more 
targeted for the system header, similar to e.g. 
http://llvm.org/viewvc/llvm-project?view=revision&revision=212238)

Thanks for the revert!

From: tha...@google.com 
[mailto:tha...@google.com] On Behalf Of Nico Weber
Sent: Thursday, May 25, 2017 9:16 AM
To: Blower, Melanie mailto:melanie.blo...@intel.com>>
Cc: rnk mailto:r...@chromium.org>>; Keane, Erich 
mailto:erich.ke...@intel.com>>; cfe-commits 
mailto:cfe-commits@lists.llvm.org>>; Hans Wennborg 
mailto:h...@chromium.org>>

Subject: RE: r303798 - For Microsoft compatibility, set fno_operator_names

In addition to this making clang-cl silently accept invalid code, it also 
breaks existing valid code, building chromium now fails. Let's revert and come 
up with something better asynchronously.

FAILED: obj/third_party/WebKit/Source/core/dom/dom/CustomElementRegistry.obj
E:\b\c\goma_client/gomacc.exe 
../../third_party/llvm-build/Release+Asserts/bin/clang-cl.exe /nologo 
/showIncludes /FC 
@obj/third_party/WebKit/Source/core/dom/dom/CustomElementRegistry.obj.rsp /c 
../../third_party/WebKit/Source/core/dom/custom/CustomElementRegistry.cpp 
/Foobj/third_party/WebKit/Source/core/dom/dom/CustomElementRegistry.obj 
/Fd"obj/third_party/WebKit/Source/core/dom/dom_cc.pdb"
E:\b\c\b\win_clang\src\third_party\WebKit\Source\core\dom\custom\CustomElementRegistry.cpp(229,7):
  error: unknown type name 'definition'
  if (definition and definition->Descriptor().LocalName() == desc.LocalName()) {
  ^
E:\b\c\b\win_clang\src\third_party\WebKit\Source\core\dom\custom\CustomElementRegistry.cpp(229,18):
  error: variable declaration in condition must have an initializer
  if (definition and definition->Descriptor().LocalName() == desc.LocalName()) {
 ^
2 errors generated.

On May 24, 2017 4:01 PM, "Blower, Melanie" 
mailto:melanie.blo...@intel.com>> wrote:
Thanks for the feedback, working on it…

From: Keane, Erich
Sent: Wednesday, May 24, 2017 3:47 PM
To: Nico Weber mailto:tha...@chromium.org>>; Blower, 
Melanie mailto:melanie.blo...@intel.com>>

Cc: cfe-commits 
mailto:cfe-commits@lists.llvm.org>>; rnk 
mailto:r...@chromium.org>>
Subject: RE: r303798 - For Microsoft compatibility, set fno_operator_names

Adding Melanie, the author of the patch.

From: tha...@google.com [mailto:tha...@google.com] On 
Behalf Of Nico Weber
Sent: Wednesday, May 24, 2017 12:43 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits 
mailto:cfe-commits@lists.llvm.org>>; rnk 
mailto:r...@chromium.org>>
Subject: Re: r303798 - For Microsoft compatibility, set fno_operator_names

Reviewed here: https://reviews.llvm.org/D33505

Still, please make this warn.

On Wed, May 24, 2017 at 3:42 PM, Nico Weber 
mailto:tha...@google.com>> wrote:
Was this reviewed somewhere?

Please make it so that this emits a warning. We want clang-cl to warn on 
invalid code (and in system headers warnings are suppressed).

On Wed, May 24, 2017 at 3:31 PM, Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Wed May 24 14:31:19 2017
New Revision: 303798

URL: http://llvm.org/viewvc/llvm-project?rev=303798&view=rev
Log:
For Microsoft compatibility, set fno_operator_names

There's a Microsoft header in the Windows SDK which won't
compile with clang because it uses an operator name (and)
as a field name. This patch allows that file to compile by
setting the option which disables operator names.
The header which doesn't compile  C:/Program Files (x86)/
Windows Kits/10/include/10.0.14393.0/um\Query.h:259:40:
error: expected member name or ';' after declaration specifiers

  /* [case()] */ NODERESTRICTION or;
   ~~~ ^

   1 error generated.

Contributed for Melanie Blower

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

Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Parser/Mic

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

2017-06-13 Thread Blower, Melanie via cfe-commits

Thanks I'll fix that. 

-Original Message-
From: Fedor Sergeev via Phabricator [mailto:revi...@reviews.llvm.org] 
Sent: Tuesday, June 13, 2017 3:30 PM
To: Blower, Melanie ; zhangsheng...@huawei.com; 
olivier...@gmail.com; kalinichev.s...@gmail.com; kf...@kde.org; 
m...@milianw.de; cfe-commits@lists.llvm.org; Keane, Erich 

Cc: kli...@google.com; simon.dar...@imgtec.com; anastasia.stul...@arm.com; 
arichardson@gmail.com; fedor.serg...@oracle.com
Subject: [PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, 
preinclude 

fedor.sergeev added a comment.

There is no /usr/include/stdc-predef.h on Solaris and I see no sense in adding 
this -include for anything except Linux (as docs are rather clear that this 
functionality is Linux-only).

Checked gcc on Solaris11/12 both SPARC and X86 - none are doing stdc-predef.h 
inclusion (checking with

  gcc  - -dI https://reviews.llvm.org/D34158



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

RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

2019-03-13 Thread Blower, Melanie via cfe-commits

Yes the IR looks a bit odd. I should use a simpler case with a global _Atomic 
int a instead of the parameter. The store is the parameter value a, if I move 
that to a global then it won't be as confusing. 

> -Original Message-
> From: JF Bastien via Phabricator [mailto:revi...@reviews.llvm.org]
> Sent: Wednesday, March 13, 2019 12:57 PM
> To: Blower, Melanie ; cfe-commits@lists.llvm.org;
> Keane, Erich ; rich...@metafoo.co.uk;
> r...@google.com; jfbast...@apple.com
> Cc: jdoerf...@anl.gov; mlek...@skidmore.edu; blitzrak...@gmail.com;
> shen...@google.com
> Subject: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of
> statement expressions
> 
> jfb requested changes to this revision.
> jfb added a comment.
> This revision now requires changes to proceed.
> 
> I think you also want to test C++ `std::atomic` as well as regular `volatile`.
> 
> 
> 
> 
> Comment at: test/Sema/atomic-expr-stmt.c:7
> +  //CHECK: %atomic-load = load atomic i32, i32* %a.addr seq_cst, align
> + 4
> +  //CHECK: store atomic i32 %atomic-load, i32* %tmp seq_cst, align 4
> +  //CHECK: %0 = load i32, i32* %tmp, align 4
> 
> Why is there a store to `a` here?
[Blower, Melanie] Yes the IR looks a bit odd. I can use a simpler case with a 
global _Atomic int a instead of the parameter. But it does seem like the "store 
atomic" to tmp shouldn't exist.  The %tmp is the value returned by the 
StmtExpr. Since the atomic load has already occurred, the value returned by 
StmtExpr could just be %atomic-load. Without the StmtExpr, there's just the 
atomic load, which is then stored into b.

I changed the test like this, and the IR follows
_Atomic int a;
void test_assign(int b) { 
  // assignment is OK
  b = ({a;}); 
}

; Function Attrs: noinline nounwind optnone
define void @test_assign(i32 %b) #0 {
entry:
  %b.addr = alloca i32, align 4
  %tmp = alloca i32, align 4
  store i32 %b, i32* %b.addr, align 4
  %atomic-load = load atomic i32, i32* @a seq_cst, align 4
  store atomic i32 %atomic-load, i32* %tmp seq_cst, align 4
  %0 = load i32, i32* %tmp, align 4
  store i32 %0, i32* %b.addr, align 4
  ret void
}
> 
> 
> 
> Comment at: test/Sema/atomic-expr-stmt.c:9
> +  //CHECK: %0 = load i32, i32* %tmp, align 4
> +  //CHECK: store i32 %0, i32* %b.addr, align 4 }
> 
> What's in `%tmp`? I'd expect the value loaded from `a` to be stored to `b`.
> 
> 
> Repository:
>   rC Clang
> 
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D59307/new/
> 
> https://reviews.llvm.org/D59307
> 
> 

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

RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

2019-03-13 Thread Blower, Melanie via cfe-commits

Sorry I formatted my reply badly, there are responses inline in the previous 
message

> -Original Message-
> From: Blower, Melanie
> Sent: Wednesday, March 13, 2019 2:06 PM
> To: 'reviews+d59307+public+153a08d52e63c...@reviews.llvm.org'
> ; cfe-
> comm...@lists.llvm.org; Keane, Erich ;
> rich...@metafoo.co.uk; r...@google.com; jfbast...@apple.com
> Cc: jdoerf...@anl.gov; mlek...@skidmore.edu; blitzrak...@gmail.com;
> shen...@google.com
> Subject: RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of
> statement expressions
> 
> Yes the IR looks a bit odd. I should use a simpler case with a global _Atomic 
> int a
> instead of the parameter. The store is the parameter value a, if I move that 
> to a
> global then it won't be as confusing.
> 
> > -Original Message-
> > From: JF Bastien via Phabricator [mailto:revi...@reviews.llvm.org]
> > Sent: Wednesday, March 13, 2019 12:57 PM
> > To: Blower, Melanie ;
> > cfe-commits@lists.llvm.org; Keane, Erich ;
> > rich...@metafoo.co.uk; r...@google.com; jfbast...@apple.com
> > Cc: jdoerf...@anl.gov; mlek...@skidmore.edu; blitzrak...@gmail.com;
> > shen...@google.com
> > Subject: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of
> > statement expressions
> >
> > jfb requested changes to this revision.
> > jfb added a comment.
> > This revision now requires changes to proceed.
> >
> > I think you also want to test C++ `std::atomic` as well as regular 
> > `volatile`.
> >
> >
> >
> > 
> > Comment at: test/Sema/atomic-expr-stmt.c:7
> > +  //CHECK: %atomic-load = load atomic i32, i32* %a.addr seq_cst,
> > + align
> > + 4
> > +  //CHECK: store atomic i32 %atomic-load, i32* %tmp seq_cst, align 4
> > +  //CHECK: %0 = load i32, i32* %tmp, align 4
> > 
> > Why is there a store to `a` here?
> [Blower, Melanie] Yes the IR looks a bit odd. I can use a simpler case with a
> global _Atomic int a instead of the parameter. But it does seem like the 
> "store
> atomic" to tmp shouldn't exist.  The %tmp is the value returned by the 
> StmtExpr.
> Since the atomic load has already occurred, the value returned by StmtExpr
> could just be %atomic-load. Without the StmtExpr, there's just the atomic 
> load,
> which is then stored into b.
> 
> I changed the test like this, and the IR follows _Atomic int a; void 
> test_assign(int
> b) {
>   // assignment is OK
>   b = ({a;});
> }
> 
> ; Function Attrs: noinline nounwind optnone define void @test_assign(i32 %b) 
> #0
> {
> entry:
>   %b.addr = alloca i32, align 4
>   %tmp = alloca i32, align 4
>   store i32 %b, i32* %b.addr, align 4
>   %atomic-load = load atomic i32, i32* @a seq_cst, align 4
>   store atomic i32 %atomic-load, i32* %tmp seq_cst, align 4
>   %0 = load i32, i32* %tmp, align 4
>   store i32 %0, i32* %b.addr, align 4
>   ret void
> }
> >
> >
> > 
> > Comment at: test/Sema/atomic-expr-stmt.c:9
> > +  //CHECK: %0 = load i32, i32* %tmp, align 4
> > +  //CHECK: store i32 %0, i32* %b.addr, align 4 }
> > 
> > What's in `%tmp`? I'd expect the value loaded from `a` to be stored to `b`.
> >
> >
> > Repository:
> >   rC Clang
> >
> > CHANGES SINCE LAST ACTION
> >   https://reviews.llvm.org/D59307/new/
> >
> > https://reviews.llvm.org/D59307
> >
> >

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

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

2017-09-12 Thread Blower, Melanie via cfe-commits



> -Original Message-
> From: Joerg Sonnenberger via Phabricator [mailto:revi...@reviews.llvm.org]
> Sent: Tuesday, September 12, 2017 3:24 PM
> To: Blower, Melanie ; olivier...@gmail.com;
> kalinichev.s...@gmail.com; kf...@kde.org; m...@milianw.de; Keane, Erich
> ; mgo...@gentoo.org; fedor.v.serg...@gmail.com;
> rich...@metafoo.co.uk; renato.go...@linaro.org
> Cc: hfin...@anl.gov; jykni...@google.com; ibiryu...@google.com; cfe-
> comm...@lists.llvm.org; kli...@google.com; simon.dar...@imgtec.com;
> anastasia.stul...@arm.com; arichardson@gmail.com
> Subject: [PATCH] D34158: For Linux/gnu compatibility, preinclude  predef.h> if the file is available
> 
> joerg added a comment.
> 
> This version is fine with me. The only contentious part is whether it should 
> be
> opt-in or opt-out for platforms, so getting this version in and revisiting 
> the issue
> again later is OK from my perspective.
> 
> 
> https://reviews.llvm.org/D34158
> 
> 
[Blower, Melanie] That's great news, thank you.  Please note there are changes 
to some clang extra tests needed with this change.  The review is here, 
https://reviews.llvm.org/D34624.  You could patch the extra tests before 
accepting the patch for D34158 (the test changes aren't dependent on D34158).  
How is platform opt-in accomplished, is it part of the configure step?

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

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

2019-07-25 Thread Blower, Melanie via cfe-commits

Thanks for your review. I put some quick replies below.  I'll work on an update 
to this. 

> -Original Message-
> From: Kevin P. Neal via Phabricator [mailto:revi...@reviews.llvm.org]
> Sent: Thursday, July 25, 2019 1:09 PM
> To: Blower, Melanie ; chandl...@gmail.com
> Cc: mgo...@gentoo.org; hiradi...@msn.com; wuz...@cn.ibm.com; Wang,
> Pengfei ; lebedev...@gmail.com;
> cameron.mcina...@nyu.edu; llvm-comm...@lists.llvm.org; Rice, Michael P
> ; Kaylor, Andrew ;
> kevin.n...@sas.com; cfe-commits@lists.llvm.org; paul.robin...@sony.com;
> peter.wal...@arm.com
> Subject: [PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-
> speculation= : specify floating point behavior
> 
> kpn added a comment.
> 
> In D62731#1601310 , @mibintc
> wrote:
> 
> > I think it would be convenient to have an "unset" setting for the different
> constrained modes, otherwise you need a boolean that says "no value was
> provided for this option".  But i'm a frontend person so I may need to have my
> attitude adjusted.
> 
> 
> What "different constrained modes"? The IRBuilder is either in constrained
> mode or it isn't. In constrained mode the exception behavior and rounding
> mode both have defaults, and those defaults can be changed individually
> without affecting the other setting. The current defaults can also be 
> retrieved if
> you need something for a function call where you don't want to change it but
> need an argument anyway. When do you need this "no value provided" setting?
[Blower, Melanie] I was thinking it would be convenient to have null values for 
RoundingMode and ExceptionBehavior but I'll work on the patch more in this area
> 
> Oh, I'd check the tools/clang/CODE_OWNERS.txt file and add additional
> appropriate reviewers. Perhaps John McCall and Richard Smith? I don't know
> who has opinions on how command line options should be handled.
[Blower, Melanie] Good idea but I'm not quite ready yet
> 
> Do we want the Unix driver to be compatible with gcc? Maybe, maybe not.
> Opinions, anyone?
[Blower, Melanie] For Intel's icc compiler, we support options describing 
fp-model in both Linux and Windows. Our customers find it useful. I want to add 
it both places. Gcc doesn't support these options.
> 
> The documentation request from lebedev.ri isn't in this ticket yet.
> 
> Also, for future historical research purposes I'd cut and paste the relevant
> portions of outside web pages (like intel.com's) and post them somewhere llvm-
> ish where they are findable. This ticket, for example, is a good place. Web 
> sites
> gets reorganized or vanish in full or in part. It's helpful to have some 
> insulation
> from that over time. I've had to fix compiler bugs that actually were 25 
> years old
> before. Yes, 25 years old. Being able to do the research is very helpful.
[Blower, Melanie] good idea, I will address that, adding it to this patch 
description? Or inline as comments in the source file itself?  (I may be able 
to compete with you on oldest bug fixed...)
> 
> Oh, it may be useful to know that constrained floating point and the
> FastMathFlags are not mutually exclusive. I don't know if that matters here or
> not, but you did mention FastMathFlags.
[Blower, Melanie] yes I worry about that
> 
> 
> 
> 
> Comment at: llvm/lib/IR/FPState.cpp:78
> +  }
> +
> +  Builder.setIsFPConstrained(IsConstrainedExcept || IsConstrainedRounding);
> 
> The IRBuilder already has defaults for exception behavior and rounding. Is it 
> a
> good idea to duplicate that knowledge here? Worse, what's here is different
> from what's in the IRBuilder. Why not ask the IRBuilder what its current 
> setting
> is and use that?
[Blower, Melanie] I was/am/ trying to translate the fp-model command line 
options semantics. I will look at this again.
> 
> Would it make sense to have setters/getters, and then have a separate
> updateBuilder() method? We still don't have a good way to get the #pragma
> down to the lower levels of clang. The current, unfinished, attempt doesn't 
> work
> for C++ templates and I'm told other cases as well. An updateBuilder() method
> could be helpful when moving from one scope to another. But keep in mind that
> if any constrained fp math is used in a function then the entire function has 
> to be
> constrained.
[Blower, Melanie] hmm
> 
> Given that last bit, it may make more sense to have the support somewhere
> higher level than as a wrapper around the IRBuilder. Maybe in
> CodeGenFunction or CodeGenModule? I've not spent much time in clang so I'm
> not sure if that makes sense or not.
> 
> 
> Repository:
>   rL LLVM
> 
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D62731/new/
> 
> https://reviews.llvm.org/D62731
> 
> 

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

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

2017-07-28 Thread Blower, Melanie via cfe-commits

 

fedor.sergeev added a comment.

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)
- 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.

>> Yes, I recognize this problem. However, I don't know an acceptable way to 
>> solve it. Does anyone have a recommendation? I had tried putting angle 
>> brackets around the file name string to imply a system-only search but that 
>> didn't work:  I guess the >angle brackets are taken to be part of the file 
>> name.  I believe it's intentional that has_include doesn't recognize the 
>> angle, I see test cases that have __has_include( "<...") Quoting from the 
>> patch:
// For standards compliance, clang will preinclude 
// -ffreestanding suppresses this behavior.
CmdArgs.push_back("-finclude-if-exists");
CmdArgs.push_back(""); // This doesn't work to restrict the 
search to system includes
  } 

>I could change the argument scanner for __has_include to recognize the angle 
>brackets -- would that be acceptable?  Alternatively, I could change the flag 
>to be "finclude-if-exists" into "fsystem-include-if-exists". Then I could 
>create a new preprocessing keyword(is that the right term?) 
>__has_system_include and use that instead of __has_include.  

>I tried this test case with -c -E:
>cat test1.c
>#if __has_include( "stdio.h" )
>#error it has stdio without angle  // This is printed
>#else
>#error it does not have stdio without angle
>#endif

>#if __has_include( "" )
>#error it has stdio with angle
>#else
>#error it does not have stdio with angle // This is printed
>#endif

  ] cat stdc-predef.h
  #error I was not expecting to see that
  ] bin/clang hello-world.c
  In file included from :2:
  ./stdc-predef.h:1:2: error: I was not expecting to see this!
  #error I was not expecting to see this!
   ^
  1 error generated.
  ]


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

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

2017-07-28 Thread Blower, Melanie 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.

>> If I make a change like this: CmdArgs.push_back(""); the 
>> program fails to pre-include std-predef.h; so I assume that there is 
>> something that needs to be fixed with this line: 
>> Opts.FIncludeIfExists.emplace_back(A->getValue());   If we want those <> in 
>> there, but no extra " in there, I would need to study which value is 
>> returned and fiddle around with the characters to make sure the right thing 
>> gets put into the input stream. This will take me quite a bit more time 
>> since I know close to diddly at this point.

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

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

2017-07-31 Thread Blower, Melanie via cfe-commits

 
fedor.sergeev added inline comments.



Comment at: test/Driver/stdc-predef.c:15
+  /* In this test, the file stdc-predef.h is missing from the 
+installation */ #if _STDC_PREDEF_H
+  #error "stdc-predef.h should not be included"

I would rather see a real check on include file inclusion (say, checking -H 
output) than a check for a macro.
Exact macro guard name is not a public interface at all. You might be lucky 
with current stdc-predef.h on Linux, but on other platforms it could be named 
differently.
> Thanks Fedor.  I am uploading a new patch with an updated version of this 
> test case. Please see comments in the new patch.

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

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

2017-08-01 Thread Blower, Melanie via cfe-commits

 
joerg added a comment.

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.
(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.
(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. 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.

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

>Thank you Joerg.  Initially I had proposed this only for gnu/Linux. I will 
>submit another patch like this.  As far as I know this is the only toolchain 
>with this behavior.  Please clarify if there are other configurations to cover.

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

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

2017-08-08 Thread Blower, Melanie via cfe-commits

 
fedor.sergeev added a comment.

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

> In fact I did have trouble writing the new test case to pass with the 
> gnu/Linux toolchain. In the file lib/Driver/ToolChains/Linux.cpp function 
> AddGnuIncludeArgs checks if GCCInstallation.isValid().


You should not be doing stdc-predef.h under GCCInstallation.isValid().
You are handling interaction with libc, so it has nothing to do with the 
presence or absence of gcc toolchain.

>> Thanks, Fedor.  I'm uploading another patch which pulls out the "isValid" 
>> check. Also I'm putting back the dummy sysroot tree which contains 
>> stdc-predef.h and adding a new test run to confirm that the new option 
>> fsystem-include-if-exists is actually working. 
https://reviews.llvm.org/D34158



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

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

2017-06-28 Thread Blower, Melanie via cfe-commits

 
Hahnfeld added a comment.

Some comments inline. In general you should consider posting an RFC on cfe-dev 
because this change will basically affect all compilations on GNU/Linux if the 
file is present.
>> Thank you

Adding Richard (general maintainer) and Renato (ARM Linux) so they are aware.




Comment at: include/clang/Driver/ToolChain.h:459-462
+  /// \brief Add arguments to use system-specific GNU includes.
+  virtual void AddGnuIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) 
+ const;
+

Why do you need to define this method in the generic `ToolChain`?
>>You're right. I don't need this. I was imitating a similar change I saw to 
>>add include directories for a different toolchain. That's why I started out 
>>this way. Thank you.


Comment at: lib/Driver/ToolChains/Gnu.cpp:2332-2349
+void Generic_GCC::addGnuIncludeArgs(const ArgList &DriverArgs, 
+ArgStringList &CC1Args) const {
+  const Generic_GCC::GCCVersion &Version = 
+GCCInstallation.getVersion();
+  if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+  !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+  !Version.isOlderThan(4, 8, 0)) {
+// If stdc-predef.h exists in the sytem includes, then -include it.

If this is GNU/Linux only, why not move to the `Linux` toolchain entirely?
>>Yes I will do that, good idea.


Comment at: lib/Driver/ToolChains/Gnu.h:328-330
+  void addGnuIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args) const;
+

This starts with a lower-case character and won't override the method in 
`ToolChain`. Adding the keyword `override` should reveal this mistake.
>>Thanks


Comment at: test/Driver/gcc-predef.c:1
+// Test that gcc preincludes stdc-predef.h on Linux //

s/gcc/clang/ ?

>>OK

Comment at: test/Driver/gcc-predef.c:9
+  #else
+#if !defined(  _STDC_PREDEF_H )
+  #error "stdc-predef.h should be preincluded for GNU/Linux 4.8 and higher"

The driver will only include `stdc-predef.h` if it can be found in the system. 
With that, the current version of this test will only run on such Linux system. 
Maybe add a basic tree in `test/Driver/Inputs` and test that the corresponding 
header is included?
>>I'm not sure what you're getting at "will only run on such a Linux system". 
>>Are you worried about the case where there's a linux system with gcc 4.8 and 
>>there's no such header? In that case this test would fail but I would think 
>>that's an outlier/oddball case. When I run this test case on my system, with 
>>that version and the file exists, this test did pass. I will study the tests 
>>in Driver/Inputs and see about creating a test there.


Comment at: test/Driver/gcc-predef.c:14-17
+// Test that gcc preincludes stdc-predef.h on Linux // // RUN: %clang 
+-c --target=i386-unknown-linux %s -Xclang -verify // 
+expected-no-diagnostics

What's the difference to the test above?
>>Whoops, I don't know how that happened. I will fix it. Thanks for your 
>>review, much appreciated.

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

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

2017-06-28 Thread Blower, Melanie via cfe-commits

 
Hahnfeld added a subscriber: cfe-commits.
Hahnfeld edited reviewers, added: rsmith, rengolin; removed: cfe-commits.
Hahnfeld added a comment.

Some comments inline. In general you should consider posting an RFC on cfe-dev 
because this change will basically affect all compilations on GNU/Linux if the 
file is present.
Adding Richard (general maintainer) and Renato (ARM Linux) so they are aware.




Comment at: include/clang/Driver/ToolChain.h:459-462
+  /// \brief Add arguments to use system-specific GNU includes.
+  virtual void AddGnuIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+  llvm::opt::ArgStringList &CC1Args) 
+ const;
+

Why do you need to define this method in the generic `ToolChain`?



Comment at: lib/Driver/ToolChains/Gnu.cpp:2332-2349
+void Generic_GCC::addGnuIncludeArgs(const ArgList &DriverArgs, 
+ArgStringList &CC1Args) const {
+  const Generic_GCC::GCCVersion &Version = 
+GCCInstallation.getVersion();
+  if (!DriverArgs.hasArg(options::OPT_ffreestanding) &&
+  !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+  !Version.isOlderThan(4, 8, 0)) {
+// If stdc-predef.h exists in the sytem includes, then -include it.

If this is GNU/Linux only, why not move to the `Linux` toolchain entirely?



Comment at: lib/Driver/ToolChains/Gnu.h:328-330
+  void addGnuIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args) const;
+

This starts with a lower-case character and won't override the method in 
`ToolChain`. Adding the keyword `override` should reveal this mistake.



Comment at: test/Driver/gcc-predef.c:1
+// Test that gcc preincludes stdc-predef.h on Linux //

s/gcc/clang/ ?



Comment at: test/Driver/gcc-predef.c:9
+  #else
+#if !defined(  _STDC_PREDEF_H )
+  #error "stdc-predef.h should be preincluded for GNU/Linux 4.8 and higher"

The driver will only include `stdc-predef.h` if it can be found in the system. 
With that, the current version of this test will only run on such Linux system. 
Maybe add a basic tree in `test/Driver/Inputs` and test that the corresponding 
header is included?
>>>OK I understand what you mean now.


Comment at: test/Driver/gcc-predef.c:14-17
+// Test that gcc preincludes stdc-predef.h on Linux // // RUN: %clang 
+-c --target=i386-unknown-linux %s -Xclang -verify // 
+expected-no-diagnostics

What's the difference to the test above?


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

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

2017-07-24 Thread Blower, Melanie via cfe-commits

@jyknight I've removed the check on nostdinc.  AFAIK, I've addressed all the 
feedback which I've received on the patch, is it OK to commit?  --Melanie

-Original Message-
From: James Y Knight via Phabricator [mailto:revi...@reviews.llvm.org] 
Sent: Monday, July 10, 2017 11:57 AM
To: Blower, Melanie ; zhangsheng...@huawei.com; 
olivier...@gmail.com; kalinichev.s...@gmail.com; kf...@kde.org; 
m...@milianw.de; Keane, Erich ; 
hahnf...@itc.rwth-aachen.de; mgo...@gentoo.org; fedor.serg...@oracle.com; 
rich...@metafoo.co.uk; renato.go...@linaro.org
Cc: jykni...@google.com; ibiryu...@google.com; cfe-commits@lists.llvm.org; 
kli...@google.com; simon.dar...@imgtec.com; anastasia.stul...@arm.com; 
arichardson@gmail.com
Subject: [PATCH] D34158: For standards compatibility, preinclude 
 if the file is available

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

RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

2018-01-11 Thread Blower, Melanie via cfe-commits



> -Original Message-
> From: Szabolcs Nagy via Phabricator [mailto:revi...@reviews.llvm.org]
> Sent: Thursday, January 11, 2018 6:13 AM
> To: Blower, Melanie ; Keane, Erich
> ; sca...@apple.com; roger.ferreriba...@arm.com;
> sjoerd.mei...@arm.com; jle...@google.com;
> hubert.reinterpretc...@gmail.com
> Cc: szabolcs.n...@arm.com; james.greenha...@arm.com;
> eli.fried...@gmail.com; efrie...@codeaurora.org; rich...@metafoo.co.uk;
> cfe-commits@lists.llvm.org; mlek...@skidmore.edu; jkor...@apple.com;
> shen...@google.com; t...@google.com
> Subject: [PATCH] D40673: Add _Float128 as alias to __float128 to enable
> compilations on Fedora27/glibc2-26
> 
> nsz added a comment.
> 
> if clang wants to provide _Float128 then the f128 constant suffix (specified 
> by
> TS18661-3) and __builtin_inff128, __builtin_nanf128, __builtin_nansf128,
> __builtin_huge_valf128 (gcc builtins required by math.h) need to be supported
> too.
[Blower, Melanie] You are right, there are f128 builtins missing and also we 
need the f128 literal suffix. There may be other builtins missing as well.  
Some of glibc sources refer to __FLT_EVAL_METHOD_TS_18661_3__ does clang 
support that?
> 
> as this patch is committed clang is broken (cannot use glibc headers) on any
> target where long double is quad precision (aarch64, mips64, s390, sparc64,
> riscv64) even if glibc fixes the >=gcc-7 check to something that works for 
> clang:
> either the _Float128 typedef conflicts with the definition by clang or the
> suffixes/builtins are missing.
> 
> 
> Repository:
>   rC Clang
> 
> https://reviews.llvm.org/D40673
> 
> 

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

RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

2018-01-11 Thread Blower, Melanie via cfe-commits



> -Original Message-
> From: Szabolcs Nagy via Phabricator [mailto:revi...@reviews.llvm.org]
> Sent: Thursday, January 11, 2018 6:25 AM
> To: Blower, Melanie ; Keane, Erich
> ; sca...@apple.com; roger.ferreriba...@arm.com;
> sjoerd.mei...@arm.com; jle...@google.com;
> hubert.reinterpretc...@gmail.com
> Cc: szabolcs.n...@arm.com; james.greenha...@arm.com;
> eli.fried...@gmail.com; efrie...@codeaurora.org; rich...@metafoo.co.uk;
> cfe-commits@lists.llvm.org; mlek...@skidmore.edu; jkor...@apple.com;
> shen...@google.com; t...@google.com
> Subject: [PATCH] D40673: Add _Float128 as alias to __float128 to enable
> compilations on Fedora27/glibc2-26
> 
> nsz added a comment.
> 
> also note that there is less than 3 weeks until glibc-2.27 is released, if the
> headers need a fix for clang then say so quickly
> 
> i opened https://sourceware.org/bugzilla/show_bug.cgi?id=22700 but it needs
> attention from some clang developers, in particular there is no way to check 
> if
> the compiler has _Float128 type defined but no working f128 suffix in the
> headers.
[Blower, Melanie] Thanks for opening this request.  Perhaps we need to pull out 
this patch and only put it back in when all the necessary builtin's and f128 
literal suffix is available.
> 
> 
> Repository:
>   rC Clang
> 
> https://reviews.llvm.org/D40673
> 
> 

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

RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

2018-01-13 Thread Blower, Melanie via cfe-commits



> -Original Message-
> From: Szabolcs Nagy via Phabricator [mailto:revi...@reviews.llvm.org]
> Sent: Friday, January 12, 2018 3:18 PM 
> 
> nsz added a comment.
>  
> 
> it is not clear to me from the original bug report what "fedora 27 workloads"
> break because of the lack of _Float128 type on x86.  The glibc headers refer 
> to
> _Float128, but normal include should not break unless the compiler claims to 
> be
> >=gcc-7 or somebody explicitly requested the _Float128 support.
[Blower, Melanie] I was building open source projects like ffmpeg, linux from 
scratch, and others. I was incorrect in my original post, I was using a patched 
compiler which led to _Float128 being exposed. In an unpatched clang this 
problem doesn't arise.  I regret the lack of care taken in my analysis of the 
problem.
> 
> if clang defines _Float128 then the headers "work" on x86 as in simple math.h
> include is not broken, but some macro definitions will use the f128 const 
> suffix
> (e.g. FLT128_MAX) which won't cause much breakage now but in the future
> users will want to know whether they can use these macros or not.
> 
> so either clang have to introduce all these features that are used by glibc
> together or provide ways for users (and glibc) to figure out what is supported
> and what isn't.
> 
> on non-x86 targets a glibc fix can solve the problem such that they "work" on
> the same level as x86, i.e. no immediate build breakage on most code, but some
> features are not supported. i think that's the right way forward, but it's 
> not clear
> if anybody has time to do the header changes in glibc before release (it has 
> to be
> tested on several targets).
> 
> if glibc is released as is today then those non-x86 targets don't want a 
> _Float128
> definition in clang-6 that breaks any math.h include on a glibc-2.27 system.
> 
> > (We have a bit of time before the 6.0 release, so we can adjust the behavior
> here to make it work.  We probably don't want to try to add full _Float128
> support on the branch, though.)
> 
> 
> 
> 
> Repository:
>   rC Clang
> 
> https://reviews.llvm.org/D40673
> 
> 

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

RE: D26636: patch for llvm/clang bug 25965, suppress warning diagnostic on float-to-bool conversion when in condition context

2016-11-18 Thread Blower, Melanie via cfe-commits

Thanks for your question

The bug was originally posted in llvm bugs database, see 25965, there is 
discussion between Richard Smith and the submitter Gregory Pakosz which 
concludes "maybe we should suppress the diagnostic entirely".

Evidently there is fairly usage to guard against divide by zero like this:

if (f)  e/f; 

The spec warning in particular is driving the interest from Intel since it's 
difficult to get the spec source code changed

--Melanie

-Original Message-
From: Joerg Sonnenberger [mailto:jo...@netbsd.org] 
Sent: Friday, November 18, 2016 11:16 AM
To: Blower, Melanie ; rich...@metafoo.co.uk; Keane, 
Erich ; david.majne...@gmail.com
Cc: cfe-commits@lists.llvm.org
Subject: [PATCH] D26636: patch for llvm/clang bug 25965, suppress warning 
diagnostic on float-to-bool conversion when in condition context

joerg added a comment.

Besides "it triggers on SPEC", why should the warning be disabled here? I think 
it is perfectly sensible to have a warning for all six conditional contexts 
here.


https://reviews.llvm.org/D26636



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

RE: r303798 - For Microsoft compatibility, set fno_operator_names

RE: r303798 - For Microsoft compatibility, set fno_operator_names

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

RE: [PATCH] D59307: Patch llvm bug 41033 concerning atomicity of statement expressions

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

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

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

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

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

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

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

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

RE: D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

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

RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

RE: D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

RE: D26636: patch for llvm/clang bug 25965, suppress warning diagnostic on float-to-bool conversion when in condition context

19 matches

Site Navigation

Mail list logo

Footer information