Re: r251738 - [MSVC Compat] Permit conversions from pointer-to-function to pointer-to-object iff -fms-compatibility

[James Dennett via cfe-commits]

On Sat, Oct 31, 2015 at 5:50 PM, Nico Weber via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> I think we more commonly say "function pointer":
>
> $ grep 'pointer-to-function' include/clang/Basic/Diagnostic*td | wc -l
>3
> $ grep 'function pointer' include/clang/Basic/Diagnostic*td | wc -l
>7
>
> For "object pointer" and "pointer-to-object" it's currently a tie. For
> "member pointer" and "pointer-to-member", the former is more common too. We
> should probably make all of these consistent – any preferences? "foo
> pointer" reads easier to me than "pointer-to-foo", but I'm not a native
> speaker.
>

The C++ Standard has some unfortunate terminology here:

   1.

   The type of a pointer to void or a pointer to an object type is called
   an object pointer type. [ Note: A pointer to void does not have a
   pointer-to-object type, however, because void is not an object type. —
   end note ]

That makes standardizing on either problematic, because they have different
meanings (any pointer-to-object type is an object pointer type, but not
vice versa).

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

Re: [PATCH] D14205: [x86] Front-end part of MCU psABI support

[Michael Kuperstein via cfe-commits]

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

LGTM



Comment at: lib/Basic/Targets.cpp:2327
@@ -2325,3 +2326,3 @@
 // X87 evaluates with 80 bits "long double" precision.
 return SSELevel == NoSSE ? 2 : 0;
   }

Do you know if we should also change this to be 0?


http://reviews.llvm.org/D14205



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

[PATCH] D14215: Disable frame pointer elimination when using -pg

[Stefan Kempf via cfe-commits]

sisnkemp created this revision.
sisnkemp added a subscriber: cfe-commits.

When using -pg in combination with optimizations (e.g. -O3), compiled programs 
crash in the profiling routine mcount().
This is because mcount (on x86_64 at least) relies on the frame pointer to be 
valid. See test case to reproduce.

This diff makes sure that the driver does not pass -fomit-frame-pointer or 
-momit-leaf-frame-pointer to the frontend when -pg is used. Currently, clang 
gives an error if -fomit-frame-pointer is used in combination with -pg, but 
-momit-leaf-frame-pointer was forgotten.
Also, disable frame pointer elimination in the frontend when -pg is set.

The behavior is consistent with gcc now: clang produces an error if 
-fomit-frame-pointer is used with -pg, and silently disables 
-momit-leaf-frame-pointer when -pg is used.

http://reviews.llvm.org/D14215

Files:
  lib/Driver/Tools.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/x86_64-profiling-keep-fp.c

Index: test/CodeGen/x86_64-profiling-keep-fp.c
===
--- /dev/null
+++ test/CodeGen/x86_64-profiling-keep-fp.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O3 -pg -S -o - %s | \
+// RUN:   FileCheck %s
+
+// Test that the frame pointer is kept when compiling with
+// profiling.
+
+//CHECK: pushq %rbp
+int main(void)
+{
+  return 0;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -450,7 +450,8 @@
   Opts.CXXCtorDtorAliases = Args.hasArg(OPT_mconstructor_aliases);
   Opts.CodeModel = getCodeModel(Args, Diags);
   Opts.DebugPass = Args.getLastArgValue(OPT_mdebug_pass);
-  Opts.DisableFPElim = Args.hasArg(OPT_mdisable_fp_elim);
+  Opts.DisableFPElim =
+  (Args.hasArg(OPT_mdisable_fp_elim) || Args.hasArg(OPT_pg));
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls);
   Opts.FloatABI = Args.getLastArgValue(OPT_mfloat_abi);
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2716,6 +2716,8 @@
   if (Arg *A = Args.getLastArg(options::OPT_fno_omit_frame_pointer,
options::OPT_fomit_frame_pointer))
 return A->getOption().matches(options::OPT_fno_omit_frame_pointer);
+  if (Args.hasArg(options::OPT_pg))
+return true;
 
   return shouldUseFramePointerForTarget(Args, Triple);
 }
@@ -2725,6 +2727,8 @@
   if (Arg *A = Args.getLastArg(options::OPT_mno_omit_leaf_frame_pointer,
options::OPT_momit_leaf_frame_pointer))
 return A->getOption().matches(options::OPT_mno_omit_leaf_frame_pointer);
+  if (Args.hasArg(options::OPT_pg))
+return true;
 
   if (Triple.isPS4CPU())
 return false;


Index: test/CodeGen/x86_64-profiling-keep-fp.c
===
--- /dev/null
+++ test/CodeGen/x86_64-profiling-keep-fp.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -O3 -pg -S -o - %s | \
+// RUN:   FileCheck %s
+
+// Test that the frame pointer is kept when compiling with
+// profiling.
+
+//CHECK: pushq %rbp
+int main(void)
+{
+  return 0;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -450,7 +450,8 @@
   Opts.CXXCtorDtorAliases = Args.hasArg(OPT_mconstructor_aliases);
   Opts.CodeModel = getCodeModel(Args, Diags);
   Opts.DebugPass = Args.getLastArgValue(OPT_mdebug_pass);
-  Opts.DisableFPElim = Args.hasArg(OPT_mdisable_fp_elim);
+  Opts.DisableFPElim =
+  (Args.hasArg(OPT_mdisable_fp_elim) || Args.hasArg(OPT_pg));
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DisableTailCalls = Args.hasArg(OPT_mdisable_tail_calls);
   Opts.FloatABI = Args.getLastArgValue(OPT_mfloat_abi);
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -2716,6 +2716,8 @@
   if (Arg *A = Args.getLastArg(options::OPT_fno_omit_frame_pointer,
options::OPT_fomit_frame_pointer))
 return A->getOption().matches(options::OPT_fno_omit_frame_pointer);
+  if (Args.hasArg(options::OPT_pg))
+return true;
 
   return shouldUseFramePointerForTarget(Args, Triple);
 }
@@ -2725,6 +2727,8 @@
   if (Arg *A = Args.getLastArg(options::OPT_mno_omit_leaf_frame_pointer,
options::OPT_momit_leaf_frame_pointer))
 return A->getOption().matches(options::OPT_mno_omit_leaf_frame_pointer);
+  if (Args.hasArg(options::OPT_pg))
+return true;
 
   if (Triple.isPS4CPU())
 return false;
___
cfe-commits mailing list

Re: r251738 - [MSVC Compat] Permit conversions from pointer-to-function to pointer-to-object iff -fms-compatibility

[Nico Weber via cfe-commits]

On Sun, Nov 1, 2015 at 12:01 AM, James Dennett 
wrote:

> On Sat, Oct 31, 2015 at 5:50 PM, Nico Weber via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I think we more commonly say "function pointer":
>>
>> $ grep 'pointer-to-function' include/clang/Basic/Diagnostic*td | wc -l
>>3
>> $ grep 'function pointer' include/clang/Basic/Diagnostic*td | wc -l
>>7
>>
>> For "object pointer" and "pointer-to-object" it's currently a tie. For
>> "member pointer" and "pointer-to-member", the former is more common too. We
>> should probably make all of these consistent – any preferences? "foo
>> pointer" reads easier to me than "pointer-to-foo", but I'm not a native
>> speaker.
>>
>
> The C++ Standard has some unfortunate terminology here:
>
>1.
>
>The type of a pointer to void or a pointer to an object type is called
>an object pointer type. [ Note: A pointer to void does not have a
>pointer-to-object type, however, because void is not an object type. —
>end note ]
>
> That makes standardizing on either problematic, because they have
> different meanings (any pointer-to-object type is an object pointer type,
> but not vice versa).
>

That distinction is probably lost on 99% of clang users though – in
general, I think clang tries to not use standardese in its diagnostics too
much (there are e.g. 0 diagnostics that mention either of prvalue, xvalue,
glvalue).

Also, since this diagnostic here talks about conversion of function
pointers to void pointers, "pointer-to-object" looks like the wrong term to
use in this case even if we tried to stick with the standard's wording,
right?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13854: Template class: emit better diagnostic in case of missing template argument list

[Davide Italiano via cfe-commits]

davide added a comment.

Gentle ping.


Repository:
  rL LLVM

http://reviews.llvm.org/D13854



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

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

[Manuel Klimek via cfe-commits]

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

lg


http://reviews.llvm.org/D14145



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

Re: r251713 - CGExprConstant.cpp: Appease Modules.

[NAKAMURA Takumi via cfe-commits]

PR25368, https://llvm.org/bugs/show_bug.cgi?id=25368
Thanks!

On Sat, Oct 31, 2015 at 2:30 AM NAKAMURA Takumi 
wrote:

> I know. The builder was red for weeks (or months).
> Please be patient for my work. It's green now.
>
> I'll file bugs, two issues,  when I got a slack time.
>
> 2015年10月30日(金) 9:44 Richard Smith :
>
>> On Oct 30, 2015 6:39 AM, "NAKAMURA Takumi via cfe-commits" <
>> cfe-commits@lists.llvm.org> wrote:
>> >
>> > Author: chapuni
>> > Date: Fri Oct 30 11:37:27 2015
>> > New Revision: 251713
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=251713&view=rev
>> > Log:
>> > CGExprConstant.cpp: Appease Modules.
>>
>> Why? Please don't work around modules bugs to make the buildbot green;
>> the purpose of the bot is to find those bugs.
>>
>> > Modified:
>> > cfe/trunk/lib/CodeGen/CGExprConstant.cpp
>> >
>> > Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=251713&r1=251712&r2=251713&view=diff
>> >
>> ==
>> > --- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
>> > +++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Fri Oct 30 11:37:27 2015
>> > @@ -1038,7 +1038,7 @@ public:
>> >unsigned Type = cast(E)->getIdentType();
>> >if (CGF) {
>> >  LValue Res =
>> CGF->EmitPredefinedLValue(cast(E));
>> > -return cast(Res.getAddress());
>> > +return llvm::cast(Res.getAddress());
>> >} else if (Type == PredefinedExpr::PrettyFunction) {
>> >  return CGM.GetAddrOfConstantCString("top level", ".tmp");
>> >}
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

[Manuel Klimek via cfe-commits]

klimek added a subscriber: klimek.
klimek added a comment.

Is this still the most current patch out there?


http://reviews.llvm.org/D5767



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

Re: r250577 - [modules] Allow the error when explicitly loading an incompatible module file

[Manuel Klimek via cfe-commits]

On Fri, Oct 23, 2015 at 9:31 PM Sean Silva  wrote:

> On Tue, Oct 20, 2015 at 1:52 AM, Manuel Klimek  wrote:
>
>> On Tue, Oct 20, 2015 at 10:41 AM Sean Silva 
>> wrote:
>>
>>> On Tue, Oct 20, 2015 at 1:38 AM, Manuel Klimek 
>>> wrote:
>>>
 On Tue, Oct 20, 2015 at 5:52 AM Sean Silva 
 wrote:

> On Mon, Oct 19, 2015 at 2:10 AM, Manuel Klimek 
> wrote:
>
>> On Sat, Oct 17, 2015 at 3:41 AM Richard Smith via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> On Fri, Oct 16, 2015 at 6:30 PM, Sean Silva 
>>> wrote:
>>>
 On Fri, Oct 16, 2015 at 6:26 PM, Richard Smith <
 rich...@metafoo.co.uk> wrote:

> On Fri, Oct 16, 2015 at 6:25 PM, Sean Silva  > wrote:
>
>> On Fri, Oct 16, 2015 at 6:12 PM, Richard Smith <
>> rich...@metafoo.co.uk> wrote:
>>
>>> On Fri, Oct 16, 2015 at 4:43 PM, Sean Silva <
>>> chisophu...@gmail.com> wrote:
>>>
 On Fri, Oct 16, 2015 at 4:20 PM, Richard Smith via cfe-commits
  wrote:

> Author: rsmith
> Date: Fri Oct 16 18:20:19 2015
> New Revision: 250577
>
> URL: http://llvm.org/viewvc/llvm-project?rev=250577&view=rev
> Log:
> [modules] Allow the error when explicitly loading an
> incompatible module file
> via -fmodule-file= to be turned off; in that case, just
> include the relevant
> files textually. This allows module files to be
> unconditionally passed to all
> compile actions via CXXFLAGS, and to be ignored for rules that
> specify custom
> incompatible flags.
>

 What direction are you trying to go with this? Are you thinking
 something like having CMake build a bunch of modules up front?

>>>
>>> That's certainly one thing you can do with this. Another is that
>>> you can make cmake automatically and explicitly build a module for 
>>> each
>>> library, and then provide that for all the dependencies of that 
>>> library,
>>>
>>
>> How does CMake know which headers are part of which library?
>> Strategically named top-level modules in the module map?
>>
>
> The idea would be for CMake to generate the module map itself
> based on the build rules.
>

 How would it know which headers to include? Do
 our ADDITIONAL_HEADER_DIRS things in our CMakeLists.txt have enough
 information for this?

>>>
>>> Some additional information may need to be added to the CMakeLists
>>> to enable this. Some build systems already model the headers for a 
>>> library,
>>> and so already have the requisite information.
>>>
>>
>> CMake supports specifying headers for libraries (mainly used for MS
>> VS). If we need this for modules, we'll probably need to update our build
>> rules (which will probably make sense anyway, for a better experience for
>> VS users ;)
>>
>
> Nice.
>
> Brad, do you have any idea how hard it would be to get cmake to
> generate clang module map files and add explicit module build steps?
> Basically, the requirements (off the top of my head) are:
> - for each library, generate a module map which is essentially just a
> list of the headers in that library (it's not just a flat list, but that's
> the gist of it).
> - for each module map, add a build step that invokes clang on it to
> say "build the module corresponding to this module map" (it's basically
> `clang++ path/to/foo.modulemap -o foo.pcm` with a little bit of fluff
> around it). There is also a dependency from foo.pcm on each of the
> libraries that library "foo" depends on.
> - for each library $Dep that library $Lib depends on, add $Dep's .pcm
> file as a dependency of the .o build steps for $Lib. $Dep's .pcm file also
> needs to be passed on the command line of the .o build steps for $Lib.
>
> It seems like similar requirements are going to be common in the
> standardized modules feature (except for the module map I think? 
> Richard?).
> Basically, in order to avoid redundantly parsing textual headers, you need
> to run a build step on headers that turns them into some form that can be
> processed more efficiently than just parsing it. E.g. the build step on
> slide 36 of this cppcon presentation about the Microsoft experimental
> modules implementation https://www.youtube.com/watch?v=RwdQA0pGWa4
> (slides: https://goo.gl/t4Eg89 ).
>
> Let me know if there is anything I can do to help (up to and including
> patches, but I'll need pointers and possibly some hand-holding as I'm
> unfam

Re: [PATCH] D13973: CFG: Delay creating Dtors for CompoundStmts which end in ReturnStmt

[Manuel Klimek via cfe-commits]

klimek added inline comments.


Comment at: lib/Analysis/CFG.cpp:1949-1952
@@ +1948,6 @@
+  }
+  if(!C->body_empty() && !dyn_cast(*C->body_rbegin())) {
+// If the body ends with a ReturnStmt, the dtors will be added in 
VisitReturnStmt
+addAutomaticObjDtors(ScopePos, scopeBeginPos, C);
+  }
+

klimek wrote:
> majnemer wrote:
> > mgehre wrote:
> > > klimek wrote:
> > > > If the body is non-empty, but the return is not the last statement, 
> > > > won't that still call addAutomaticObjDtors twice?
> > > Yes, if there are statements after a "return" (i.e. dead code), it will 
> > > still call addAutomaticObjDtors  twice, just like before this patch.
> > > I guess that this case is not to common, and would be flagged by other 
> > > checks.
> > Please don't use `dyn_cast` if you just need to perform a type test, use 
> > `isa` instead.
> > You need a space between `if` and `(`.
> Is there a reason to not fix that while we're here (seems like a related 
> issue into which somebody might run down the road otherwise?)
I'd still be curious about an answer to my question :)


http://reviews.llvm.org/D13973



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

Re: [libcxx] r249929 - Split out of .

[Michael Zolotukhin via cfe-commits]

Thanks, Richard! Just to confirm, the test is passing now.

Michael

> On Oct 29, 2015, at 5:22 PM, Richard Smith  wrote:
> 
> I reverted this change in r251665, and started a new thread for the patch to 
> reinstate this and fix the ambiguity issue.
> 
> On Wed, Oct 28, 2015 at 11:01 AM, Michael Zolotukhin via cfe-commits 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Hi Eric, Richard,
> 
> Any news on this? The test is still broken, should we revert the commit for 
> now?
> 
> Thanks,
> Michael
> 
>> On Oct 24, 2015, at 1:18 AM, Eric Fiselier > > wrote:
>> 
>> Hi Michael,
>> 
>> Sorry I'm holding this patch up in review. The fix is quite "novel" and I 
>> want to make sure we get it right. If we can't land it over the weekend I'll 
>> ask Richard to revert while we work on it.
>> 
>> /Eric
>> 
>> On Oct 23, 2015 10:13 PM, "Michael Zolotukhin via cfe-commits" 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> Hi Richard,
>> 
>> Is this patch ready for commit, or were you just checking an idea? Our bots 
>> are still failing to build povray, so we’re really looking forward for some 
>> fix:)
>> 
>> Thanks,
>> Michael
>> 
>>> On Oct 15, 2015, at 6:21 PM, Manman Ren via cfe-commits 
>>> mailto:cfe-commits@lists.llvm.org>> wrote:
>>> 
 
 On Oct 15, 2015, at 1:41 PM, Richard Smith >>> > wrote:
 
 On Thu, Oct 15, 2015 at 12:03 PM, Manman Ren via cfe-commits 
 mailto:cfe-commits@lists.llvm.org>> wrote:
 
> On Oct 15, 2015, at 11:25 AM, Richard Smith  > wrote:
> 
> I assume the code in question has a "using namespace std;"?
> 
> 
 Yes
 
> I don't see any way around this other than giving up on trying to fix the 
> function signatures here (or maybe adding a Clang feature to let us fix 
> the bad signature).
> 
> 
 Can you elaborate on how to fix the bad signature by adding a Clang 
 feature? I want to see how hard it is before giving up on trying to fix 
 the signatures.
 
 I thought about this a bit more, and we already have a feature that can be 
 used for this.
 
 Please let me know if the attached patch resolves the issue for you. This 
 should also fix the wrong overload sets for these functions being provided 
 by  on Darwin.
>>> 
>>> This works on my testing case. Thanks!!
>>> 
>>> Manman
>>> 
 
 
 Eric, Marshall: the attached patch adds a macro _LIBCPP_PREFERRED_OVERLOAD 
 that can be applied to a function to (a) mark it as a separate overload 
 from any other function with the same signature without the overload, and 
 (b) instruct the compiler that it's preferred over another function with 
 the same signature without the attribute. This allows us to replace the 
 libc function
 
   char *strchr(const char *, int);
 
 with the C++ overload set:
 
   const char *strchr(const char *, int);
   char *strchr(char *, int);
 
 It only works with Clang, though; for other compilers, we leave the C 
 library's signature alone (as we used to before my patches landed).
 
 Thanks,
 Manman
 
 
> On Oct 15, 2015 11:07 AM, "Manman Ren via cfe-commits" 
> mailto:cfe-commits@lists.llvm.org>> wrote:
> Hi Richard,
> 
> This is causing a failure when building povray on iOS.
> 
> Compilation error:
> /Users/buildslave/tmp/test-suite-externals/speccpu2006/benchspec/CPU2006/453.povray/src/fileinputoutput.cpp:364:20:
>  error: call to 'strrchr' is ambiguous
>  const char *p=strrchr(name, '.’);
> 
> iOS.sdk/usr/include/string.h:87:7: note: candidate function
> char*strrchr(const char *, int);
>  ^
> /Users/buildslave/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../include/c++/v1/cstring:109:46:
>  note: candidate function
> inline _LIBCPP_INLINE_VISIBILITY const char* strrchr(const char* __s, int 
> __c) {return ::strrchr(__s, __c);}
> 
> It is a little strange to have "char*strrchr(const char *, int);” in 
> iOS. But it is already in our SDK.
> 
> Do you have any suggestion on how to fix this?
> 
> Thanks,
> Manman
> 
> > On Oct 9, 2015, at 6:25 PM, Richard Smith via cfe-commits 
> > mailto:cfe-commits@lists.llvm.org>> wrote:
> >
> > Author: rsmith
> > Date: Fri Oct  9 20:25:31 2015
> > New Revision: 249929
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=249929&view=rev 
> > 
> > Log:
> > Split  out of .
> >
> > Also fix the overload set for the five functions whose signatures 
> > change in the
> > case where we can fix it. This is already covered by existing tests for 
> > the
> > affected systems.
> >
> > Added:
> >libcxx/trunk/incl

Re: [PATCH] D14096: [clang-tidy] add new check cppcoreguidelines-pro-type-cstyle-cast

[Joerg Sonnenberger via cfe-commits]

On Tue, Oct 27, 2015 at 06:10:26PM +, Samuel Benzaquen via cfe-commits 
wrote:
> sbenza added a comment.
> 
> In http://reviews.llvm.org/D14096#275902, @xazax.hun wrote:
> 
> > There is already a similar check in the Google package. What are the 
> > differences between those two checks? What is the reason we can not just 
> > register that check into the core guidelines module?
> 
> 
> That other check discourages c-style cast in favor of C++ style casts, even 
> if it is a reinterpret_cast. It simply replaces the cstyle cast with an 
> equivalent C++ one. It is basically a stylistic check.
> 
> This check will warn unsafe cstyle casts, while allowing safe ones like 
> int->uint casts.
> This one is a safety related check.

Looking back to the discussion about the C++ style casts, this argument
makes no sense. For C++ code, reinterpret_cast is clearly preferable
over C-style casts for all but code size reasons. There seems to be no
consideration about "safe" uses with reinterpret_cast, so why should C-style 
casts
be different?

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

[libcxx] r251767 - Improve the tests for 'is_literal_type'

[Marshall Clow via cfe-commits]

Author: marshall
Date: Sun Nov  1 15:13:10 2015
New Revision: 251767

URL: http://llvm.org/viewvc/llvm-project?rev=251767&view=rev
Log:
Improve the tests for 'is_literal_type'

Modified:

libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_literal_type.pass.cpp

Modified: 
libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_literal_type.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_literal_type.pass.cpp?rev=251767&r1=251766&r2=251767&view=diff
==
--- 
libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_literal_type.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/meta/meta.unary/meta.unary.prop/is_literal_type.pass.cpp
 Sun Nov  1 15:13:10 2015
@@ -12,17 +12,20 @@
 // is_literal_type
 
 #include 
+#include "test_macros.h"
 
 template 
 void test_is_literal_type()
 {
 static_assert( std::is_literal_type::value, "");
+static_assert( std::is_literal_type::value, "");
+static_assert( std::is_literal_type::value, "");
+static_assert( std::is_literal_type::value, "");
 #if TEST_STD_VER > 14
 static_assert( std::is_literal_type_v, "");
-// static_assert( std::is_final_v, "");
-// static_assert( std::is_final_v, "");
-// static_assert( std::is_final_v, "");
-// static_assert( std::is_final_v, "");
+static_assert( std::is_literal_type_v, "");
+static_assert( std::is_literal_type_v, "");
+static_assert( std::is_literal_type_v, "");
 #endif
 }
 
@@ -30,31 +33,72 @@ template 
 void test_is_not_literal_type()
 {
 static_assert(!std::is_literal_type::value, "");
+static_assert(!std::is_literal_type::value, "");
+static_assert(!std::is_literal_type::value, "");
+static_assert(!std::is_literal_type::value, "");
 #if TEST_STD_VER > 14
-static_assert( std::is_literal_type_v, "");
-// static_assert( std::is_final_v, "");
-// static_assert( std::is_final_v, "");
-// static_assert( std::is_final_v, "");
-// static_assert( std::is_final_v, "");
+static_assert(!std::is_literal_type_v, "");
+static_assert(!std::is_literal_type_v, "");
+static_assert(!std::is_literal_type_v, "");
+static_assert(!std::is_literal_type_v, "");
 #endif
 }
 
-struct A
+class Empty
+{
+};
+
+class NotEmpty
+{
+virtual ~NotEmpty();
+};
+
+union Union {};
+
+struct bit_zero
 {
+int :  0;
 };
 
-struct B
+class Abstract
 {
-B();
+virtual ~Abstract() = 0;
 };
 
+enum Enum {zero, one};
+
+typedef void (*FunctionPtr)();
+
 int main()
 {
-test_is_literal_type ();
-test_is_literal_type ();
-test_is_literal_type ();
-test_is_literal_type ();
-test_is_literal_type ();
+#if TEST_STD_VER >= 11
+test_is_literal_type();
+#endif
+
+// Before C++14, void was not a literal type
+// In C++14, cv-void is is a literal type
+#if TEST_STD_VER < 14
+test_is_not_literal_type();
+#else TEST_STD_VER > 14
+test_is_literal_type();
+#endif
+
+test_is_literal_type();
+test_is_literal_type();
+test_is_literal_type();
+test_is_literal_type();
+#if TEST_STD_VER >= 11
+test_is_literal_type();
+#endif
+test_is_literal_type();
+test_is_literal_type();
+test_is_literal_type();
+test_is_literal_type();
+test_is_literal_type();
+test_is_literal_type();
+test_is_literal_type();
+test_is_literal_type();
 
-test_is_not_literal_type ();
+test_is_not_literal_type();
+test_is_not_literal_type();
 }


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

[libcxx] r251768 - Add 'nostdinc++' to the flags used by testit. Makes the tests run better on Mac OS X with the new depr.c headers change

[Marshall Clow via cfe-commits]

Author: marshall
Date: Sun Nov  1 15:14:22 2015
New Revision: 251768

URL: http://llvm.org/viewvc/llvm-project?rev=251768&view=rev
Log:
Add 'nostdinc++' to the flags used by testit. Makes the tests run better on Mac 
OS X with the new depr.c headers change

Modified:
libcxx/trunk/test/testit

Modified: libcxx/trunk/test/testit
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/testit?rev=251768&r1=251767&r2=251768&view=diff
==
--- libcxx/trunk/test/testit (original)
+++ libcxx/trunk/test/testit Sun Nov  1 15:14:22 2015
@@ -42,7 +42,7 @@ then
 fi
 if [ -z "$OPTIONS" ]
 then
-   OPTIONS="-std=${CXX_LANG} -stdlib=libc++"
+   OPTIONS="-std=${CXX_LANG} -stdlib=libc++ -nostdinc++"
 fi
 OPTIONS="$OPTIONS -I$LIBCXX_ROOT/test/support"
 


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

Re: [PATCH] D13973: CFG: Delay creating Dtors for CompoundStmts which end in ReturnStmt

[Matthias Gehre via cfe-commits]

mgehre added inline comments.


Comment at: lib/Analysis/CFG.cpp:1949-1952
@@ +1948,6 @@
+  }
+  if (!C->body_empty() && !isa(*C->body_rbegin())) {
+// If the body ends with a ReturnStmt, the dtors will be added in 
VisitReturnStmt
+addAutomaticObjDtors(ScopePos, scopeBeginPos, C);
+  }
+

Yes, sorry, I was busy with something else. I was reluctant to fix the more 
general case, because that would introduce a second loop, and may (or may not) 
have performance implications. On the other hand, unreachable block don't seem 
to have any negative impact.



http://reviews.llvm.org/D13973



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

Re: [PATCH] D13899: Fix bug in suggested fix that truncated variable names to 1 character.

[Aaron Ballman via cfe-commits]

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

LGTM!



Comment at: clang-tidy/misc/UnusedParametersCheck.cpp:25
@@ +24,3 @@
+template 
+static CharSourceRange removeNode(const MatchFinder::MatchResult &Result,
+  const T *PrevNode, const T *Node,

Are we preferring anonymous namespaces over static functions these days?


http://reviews.llvm.org/D13899



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

Re: [PATCH] D14096: [clang-tidy] add new check cppcoreguidelines-pro-type-cstyle-cast

[Matthias Gehre via cfe-commits]

mgehre updated this revision to Diff 38857.
mgehre added a comment.

Review comments: Formating and changed diag messages


http://reviews.llvm.org/D14096

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.cpp
  clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-pro-type-cstyle-cast.cpp

Index: test/clang-tidy/cppcoreguidelines-pro-type-cstyle-cast.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-pro-type-cstyle-cast.cpp
@@ -0,0 +1,141 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-cstyle-cast %t
+
+void reinterpretcast() {
+  int i = 0;
+  void *j;
+  j = (int*)j;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use c-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
+}
+
+void constcast() {
+  int* i;
+  const int* j;
+  i = (int*)j;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use c-style cast to cast away constness [cppcoreguidelines-pro-type-cstyle-cast]
+  j = (const int*)i; // OK, const added
+  (void)j; // OK, not a const_cast
+}
+
+void const_and_reinterpret() {
+  int* i;
+  const void* j;
+  i = (int*)j;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: do not use c-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast]
+}
+
+class Base {
+};
+
+class Derived : public Base {
+};
+
+class Base2 {
+};
+
+class MultiDerived : public Base, public Base2 {
+};
+
+class PolymorphicBase {
+public:
+  virtual ~PolymorphicBase();
+};
+
+class PolymorphicDerived : public PolymorphicBase {
+};
+
+class PolymorphicMultiDerived : public Base, public PolymorphicBase {
+};
+
+void pointers() {
+
+  auto P0 = (Derived*)new Base();
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use c-style cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-cstyle-cast]
+
+  const Base* B0;
+  auto PC0 = (const Derived*)(B0);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use c-style cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-cstyle-cast]
+
+  auto P1 = (Base*)new Derived(); // OK, upcast to a public base
+  auto P2 = (Base*)new MultiDerived(); // OK, upcast to a public base
+  auto P3 = (Base2*)new MultiDerived(); // OK, upcast to a public base
+}
+
+void pointers_polymorphic() {
+
+  auto PP0 = (PolymorphicDerived*)new PolymorphicBase();
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use c-style cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-cstyle-cast]
+  // CHECK-FIXES: auto PP0 = dynamic_cast(new PolymorphicBase());
+
+  const PolymorphicBase* B0;
+  auto PPC0 = (const PolymorphicDerived*)B0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: do not use c-style cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-cstyle-cast]
+  // CHECK-FIXES: auto PPC0 = dynamic_cast(B0);
+
+
+  auto B1 = (PolymorphicBase*)new PolymorphicDerived(); // OK, upcast to a public base
+  auto B2 = (PolymorphicBase*)new PolymorphicMultiDerived(); // OK, upcast to a public base
+  auto B3 = (Base*)new PolymorphicMultiDerived(); // OK, upcast to a public base
+}
+
+void arrays() {
+  Base ArrayOfBase[10];
+  auto A0 = (Derived*)ArrayOfBase;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use c-style cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-cstyle-cast]
+}
+
+void arrays_polymorphic() {
+  PolymorphicBase ArrayOfPolymorphicBase[10];
+  auto AP0 = (PolymorphicDerived*)ArrayOfPolymorphicBase;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use c-style cast to downcast from a base to a derived class; use dynamic_cast instead
+  // CHECK-FIXES: auto AP0 = dynamic_cast(ArrayOfPolymorphicBase);
+}
+
+void references() {
+  Base B0;
+  auto R0 = (Derived&)B0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use c-style cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-cstyle-cast]
+  Base& RefToBase = B0;
+  auto R1 = (Derived&)RefToBase;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use c-style cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-cstyle-cast]
+
+  const Base& ConstRefToBase = B0;
+  auto RC1 = (const Derived&)ConstRefToBase;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use c-style cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-cstyle-cast]
+
+
+  Derived RD1;
+  auto R2 = (Base&)RD1; // OK, upcast to a public base
+}
+
+void references_polymorphic() {
+  PolymorphicBase B0;
+  auto RP0 = (PolymorphicDerived&)B0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use c-style cast to dow

Re: [PATCH] D13973: CFG: Delay creating Dtors for CompoundStmts which end in ReturnStmt

[Manuel Klimek via cfe-commits]

klimek added a reviewer: jordan_rose.
klimek added a comment.

+jordan for an opinion on whether  we want to fix the more general case.

My main problem is that while we're at it we need to fully understand the 
change anyway, and if somebody runs into this later, they'll need a long time 
debugging this surprising effect (like what happened to you here ;)


http://reviews.llvm.org/D13973



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

Re: [PATCH] D13844: [libclang] Visit TypeAliasTemplateDecl

[Benjamin Kramer via cfe-commits]

bkramer accepted this revision.
bkramer added a comment.

Looks good to me too, do you have commit access?


http://reviews.llvm.org/D13844



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

Re: Porting Clang to a new architecture

[Justin Bogner via cfe-commits]

Ariel Arelovich via cfe-commits  writes:
> Great.
>
> So this is what I wanted to know. I think understand what you are saying.
> You are saying that most of the work that I would need to do from the AST
> (I'm assuming that this stands from Abstract Syntax Tree) is allready being
> done by LLVM and I should look to modify for a my new arquitecture. Right?

This is close, but a little bit off. Clang generates an AST, but then
transforms it further into the LLVM intermediate language[1]. LLVM can
generate code for a number of target architectures from this language.
It sounds to me like what you want to do is write an LLVM backend for
your target. There is some documentation[2] on how to do that, which
should give you an idea of what you would need to do to leverage LLVM
for your project.

[1]: http://llvm.org/docs/LangRef.html
[2]: http://llvm.org/docs/WritingAnLLVMBackend.html

> If I take the AST and process it myself to generate the opcodes, I'm
> essentially writing LLVM just less efficient (no optimizations) and
> specific for the new architecure, right?
>
>
>
> On Mon, Oct 26, 2015 at 12:32 PM, Saleem Abdulrasool 
> wrote:
>
>> On Monday, October 26, 2015, Ariel Arelovich via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Hi:
>>>
>>> Where I'm working I've been asked to look into developing a C (by this I
>>> mean C89 C standard) compiler a new processor architecture being created
>>> inhouse.
>>>
>>> On looking at the options and brushing up on compiler theory, I came up
>>> with Flex/Bison, Writing the compiler from scratch or use Clang. I liked
>>> this option better because parsing and and analyzing c code to genereate
>>> somthing intermediate, and do this from scratch, seemed like reinventing
>>> the wheel (an incredibly complex wheel, at that).
>>>
>>> I've figured that if Clang, works with the standard compiler flow (that
>>> I've seen everywhere), it takes a C program and transforms it until it gets
>>> a intermediate language (which is machine independant). The Dragon Book,
>>> even calls this the output of the compiler Front End. From what I've read
>>> from the documentation This is what Clang is: a compiler front end. LLVM
>>> does optimization and translation to opcodes of known architectures. Is
>>> this correct? Or am I getting this wrong?
>>>
>>
>> Correct.  clang is just the front end for C-like languages.  It will parse
>> and analyze the code (generating the AST).  The IR it generates is the LLVM
>> IR.
>>
>> So, my question is: Would it be possible to use Clang in such a way that
>>> it takes a C program and generates an intermediate language so that I can
>>> work directly on that language to generate opcodes based on the
>>> architecture that we are currently developing?
>>>
>>
>> Yes.  This is how clang already works.  It sounds like you should be
>> looking at LLVM, not clang since you are trying to support a new
>> architecture, not a language feature.
>>
>>
>>>
>>> Thank you, for any answers.
>>>
>>> PD: I know my question might be very hard to answer as it is not very
>>> specific. All I'm asking for, here, is a simple: "It could be done, start
>>> reading about this and that" or "It will be next to impossible, just try
>>> something else" (with a little explanation as to why, if that were
>>> possible, please).
>>>
>>
>>
>> --
>> Saleem Abdulrasool
>> compnerd (at) compnerd (dot) org
>>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D10802: [mips] Interrupt attribute support.

[Aaron Ballman via cfe-commits]

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

This is coming along nicely! There are some minor nits that should be trivial 
to fix, but the fatal errors need to become semantic errors, and some tests are 
missing.



Comment at: include/clang/Basic/AttrDocs.td:713
@@ +712,3 @@
+Clang supports the GNU style ``__attribute__((interrupt("ARGUMENT")))`` 
attribute on
+MIPS targets. This attribute may be attached ot a function definition and 
instructs
+the backend to generate appropriate function entry/exit code so that it can be 
used

s/ot/to


Comment at: include/clang/Basic/AttrDocs.td:717
@@ +716,3 @@
+
+By default the compiler will produce a function prologue and epilogue suitable 
for
+an interrupt service routine that handles an External Interrupt Controller 
(eic)

Comma after "By default"


Comment at: include/clang/Basic/AttrDocs.td:722
@@ +721,3 @@
+
+Otherwise, for use with vectored interrupt mode the argument passed should be
+of the form "vector=LEVEL" where LEVEL is one of the following values:

Comma after mode.


Comment at: include/clang/Basic/AttrDocs.td:738
@@ +737,3 @@
+
+- The FPU is disabled in the prologue as the floating pointer registers are not
+  spilled to the stack.

Comma after prologue.


Comment at: include/clang/Basic/AttrDocs.td:741
@@ +740,3 @@
+
+- Function return is changed to an exception return instruction.
+

The function return?


Comment at: lib/CodeGen/TargetInfo.cpp:5834
@@ +5833,3 @@
+
+const MipsInterruptAttr * Attr = FD->getAttr();
+if (!Attr)

No space between * and Attr. May want to run clang-format over the patch.


Comment at: lib/CodeGen/TargetInfo.cpp:5839
@@ +5838,3 @@
+if (!Fn->arg_empty())
+  llvm::report_fatal_error(
+  "Functions with arguments and interrupt attribute not supported!");

This should be a semantic error for improved diagnostic behavior.


Comment at: lib/CodeGen/TargetInfo.cpp:5843
@@ +5842,3 @@
+if (Fn->hasFnAttribute("mips16"))
+  llvm::report_fatal_error("Functions with the interrupt and mips16 "
+   "attributes are not supported!");

Same here. Also, missing a semantic test for this.


Comment at: lib/Sema/SemaDeclAttr.cpp:4274
@@ +4273,3 @@
+S.Diag(Attr.getLoc(), diag::warn_attribute_type_not_supported)
+<< Attr.getName() << Str << ArgLoc;
+return;

Please have Str in quotes in the diagnostic (since it's something the user 
wrote).


Comment at: lib/Sema/SemaDeclAttr.cpp:4278
@@ +4277,3 @@
+
+  unsigned Index = Attr.getAttributeSpellingListIndex();
+  D->addAttr(::new (S.Context)

Sink Index into the constructor arguments.


Comment at: test/CodeGen/mips-interrupt-attr-arg.c:5
@@ +4,3 @@
+; CHECK: LLVM ERROR: Functions with the interrupt attribute cannot have 
arguments!
+void __attribute__ ((interrupt("vector=sw0")))
+isr_sw0 (char * a)

This should be folded into the other semantic test when this becomes a semantic 
error instead of a hard failure.


Comment at: test/Sema/mips-interrupt-attr.c:18
@@ +17,2 @@
+__attribute__((interrupt())) void fooc() {}
+__attribute__((interrupt(""))) void food() {}

Also missing semantic tests for placing the attribute on something other than a 
function (which I presume should be an error?).


http://reviews.llvm.org/D10802



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

Re: [PATCH] D12922: Add support for function attribute "notail"

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: lib/Sema/SemaDecl.cpp:5374
@@ +5373,3 @@
+
+  // Virtual functions cannot be marked as 'notail'.
+  if (auto *Attr = ND.getAttr())

Is there a reason this is here instead of SemaDeclAttr.cpp? It seems like the 
decl passed in with the attribute attached to it should already be known to be 
virtual or not?


Comment at: test/Sema/attr-notail.c:8
@@ +7,2 @@
+  return a ? callee0() : callee1();
+}

Missing a test case for the attribute being specified on something other than a 
function, and one for being passed arguments.


Comment at: test/SemaCXX/attr-notail.cpp:5
@@ +4,3 @@
+public:
+  [[clang::not_tail_called]] virtual int foo1(); // expected-error 
{{'not_tail_called' attribute cannot be applied to virtual function}}
+  virtual int foo2();

Missing the "s" on virtual functions.


http://reviews.llvm.org/D12922



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

Re: [PATCH] D13643: [Sema] Warn on ternary comparison

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

Mostly looks good; the only thing left is a test to ensure the fixits are 
placing the parens in the expected location.



Comment at: lib/Sema/SemaChecking.cpp:6725
@@ -6706,1 +6724,3 @@
 static void AnalyzeComparison(Sema &S, BinaryOperator *E) {
+  // We want to do this, no matter the types
+  DiagnoseTernaryComparison(S, E);

Missing a period to end the sentence.


http://reviews.llvm.org/D13643



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

Re: [PATCH] D14096: [clang-tidy] add new check cppcoreguidelines-pro-type-cstyle-cast

[Aaron Ballman via cfe-commits]

On Sun, Nov 1, 2015 at 2:31 PM, Joerg Sonnenberger via cfe-commits
 wrote:
> On Tue, Oct 27, 2015 at 06:10:26PM +, Samuel Benzaquen via cfe-commits 
> wrote:
>> sbenza added a comment.
>>
>> In http://reviews.llvm.org/D14096#275902, @xazax.hun wrote:
>>
>> > There is already a similar check in the Google package. What are the 
>> > differences between those two checks? What is the reason we can not just 
>> > register that check into the core guidelines module?
>>
>>
>> That other check discourages c-style cast in favor of C++ style casts, even 
>> if it is a reinterpret_cast. It simply replaces the cstyle cast with an 
>> equivalent C++ one. It is basically a stylistic check.
>>
>> This check will warn unsafe cstyle casts, while allowing safe ones like 
>> int->uint casts.
>> This one is a safety related check.
>
> Looking back to the discussion about the C++ style casts, this argument
> makes no sense. For C++ code, reinterpret_cast is clearly preferable
> over C-style casts for all but code size reasons. There seems to be no
> consideration about "safe" uses with reinterpret_cast, so why should C-style 
> casts
> be different?

"Clearly preferable" is kind of debatable, but I don't disagree with
your statement. That being said, this checker isn't concerned with
C++-style casts, so I'm not certain I understand what you would like
to see changed with this checker. Can you elaborate?

~Aaron

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

Re: [PATCH] D14096: [clang-tidy] add new check cppcoreguidelines-pro-type-cstyle-cast

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.cpp:49
@@ +48,3 @@
+diag(MatchedCast->getLocStart(),
+ "do not use c-style cast to convert between unrelated types");
+return;

"C-style" instead of "c-style"?


Comment at: clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.cpp:57
@@ +56,3 @@
+const auto *SourceDecl = SourceType->getPointeeCXXRecordDecl();
+if (!SourceDecl) // The cast is from object to reference
+  SourceDecl = SourceType->getAsCXXRecordDecl();

Missing period to terminate the comment.


Comment at: clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.cpp:74
@@ +73,3 @@
+  MatchedCast->getLocStart(),
+  "do not use c-style cast to downcast from a base to a derived class; 
"
+  "use dynamic_cast instead");

"C-style"


Comment at: clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.cpp:79
@@ +78,3 @@
+  MatchedCast->getSubExprAsWritten()->IgnoreImpCasts();
+  std::string CastText = ("dynamic_cast<" + DestTypeString + ">").str();
+  if (!isa(SubExpr)) {

How does this handle a case like:
```
SomeClass::type *derived = (SomeClass::type *)someBase;
```
Will it leave the spelling as "SomeClass::type *", or replace it with the 
underlying type from the typedef? (I suspect it leaves it as-is, but it would 
be good to have tests for more complex code patterns.)


Comment at: clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.cpp:80
@@ +79,3 @@
+  std::string CastText = ("dynamic_cast<" + DestTypeString + ">").str();
+  if (!isa(SubExpr)) {
+CastText.push_back('(');

Why do we need this check? Perhaps we want to use IgnoreParenImpCasts() instead?


Comment at: clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.cpp:94
@@ +93,3 @@
+  MatchedCast->getLocStart(),
+  "do not use c-style cast to downcast from a base to a derived 
class");
+return;

C-style


Comment at: clang-tidy/cppcoreguidelines/ProTypeCstyleCastCheck.cpp:101
@@ +100,3 @@
+diag(MatchedCast->getLocStart(),
+ "do not use c-style cast to cast away constness");
+  }

C-style


Comment at: docs/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.rst:4
@@ +3,3 @@
+
+This check flags all use of c-style casts that perform a static_cast downcast, 
const_cast, or reinterpret_cast.
+

C-style


http://reviews.llvm.org/D14096



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

Re: [PATCH] D13844: [libclang] Visit TypeAliasTemplateDecl

[Milian Wolff via cfe-commits]

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

+1 from my side, but someone else has to approve


http://reviews.llvm.org/D13844



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

r251691 - Update debug-info-scope test to remove "FIXME", which is fixed in r251689

[Dehao Chen via cfe-commits]

Author: dehao
Date: Fri Oct 30 00:08:43 2015
New Revision: 251691

URL: http://llvm.org/viewvc/llvm-project?rev=251691&view=rev
Log:
Update debug-info-scope test to remove "FIXME", which is fixed in r251689

Modified:
cfe/trunk/test/CodeGen/debug-info-scope.c

Modified: cfe/trunk/test/CodeGen/debug-info-scope.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/debug-info-scope.c?rev=251691&r1=251690&r2=251691&view=diff
==
--- cfe/trunk/test/CodeGen/debug-info-scope.c (original)
+++ cfe/trunk/test/CodeGen/debug-info-scope.c Fri Oct 30 00:08:43 2015
@@ -8,10 +8,6 @@ int main() {
 // CHECK: !DILocalVariable(name: "i"
 // CHECK-NEXT: !DILexicalBlock(
 
-// FIXME: Looks like we don't actually need both these lexical blocks (disc 2
-// just refers to disc 1, nothing actually uses disc 2).
-// GMLT-NOT: !DILexicalBlock
-// GMLT: !DILexicalBlockFile({{.*}}, discriminator: 2)
 // GMLT-NOT: !DILexicalBlock
 // GMLT: !DILexicalBlockFile({{.*}}, discriminator: 1)
 // Make sure we don't have any more lexical blocks because we don't need them 
in


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

Re: [PATCH] D14149: __builtin_signbit fix for ppcf128 type

[Aleksandar Beserminji via cfe-commits]

abeserminji updated this revision to Diff 38795.
abeserminji added a comment.

Modified comments as suggested by @rjmccall


Repository:
  rL LLVM

http://reviews.llvm.org/D14149

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/Analysis/builtin_signbit.cpp

Index: test/Analysis/builtin_signbit.cpp
===
--- test/Analysis/builtin_signbit.cpp
+++ test/Analysis/builtin_signbit.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang -target powerpc-linux-gnu -emit-llvm -S -O0 %s -o - | 
FileCheck %s --check-prefix=CHECK-BE --check-prefix=CHECK
+// RUN: %clang -target powerpc64-linux-gnu   -emit-llvm -S -O0 %s -o - | 
FileCheck %s --check-prefix=CHECK-BE --check-prefix=CHECK
+// RUN: %clang -target powerpc64le-linux-gnu -emit-llvm -S -O0 %s -o - | 
FileCheck %s --check-prefix=CHECK-LE --check-prefix=CHECK
+
+bool b;
+double d = -1.0;
+long double ld = -1.0L;
+void test_signbit()
+{
+  b = __builtin_signbit(1.0L);
+  // CHECK: i128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+
+  b = __builtin_signbit(ld);
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+
+  b = __builtin_signbitf(1.0);
+  // CHECK: store i8 0
+
+  b = __builtin_signbitf(d);
+  // CHECK: bitcast
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE-NOT: lshr
+
+  b = __builtin_signbitl(1.0L);
+  // CHECK: i128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+
+  b = __builtin_signbitl(ld);
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -238,10 +238,20 @@
   llvm::Type *IntTy = llvm::IntegerType::get(C, Width);
   V = CGF.Builder.CreateBitCast(V, IntTy);
   if (Ty->isPPC_FP128Ty()) {
-// The higher-order double comes first, and so we need to truncate the
-// pair to extract the overall sign. The order of the pair is the same
-// in both little- and big-Endian modes.
+// We want the sign bit of the higher-order double. The bitcast we just
+// did works as if the double-double was stored to memory and then
+// read as an i128. The "store" will put the higher-order double in the
+// lower address in both little- and big-Endian modes, but the "load"
+// will treat those bits as a different part of the i128: the low bits in
+// little-Endian, the high bits in big-Endian. Therefore, on big-Endian
+// we need to shift the high bits down to the low before truncating.
 Width >>= 1;
+if (CGF.getTarget().isBigEndian()) {
+  Value *ShiftCst = llvm::ConstantInt::get(IntTy, Width);
+  V = CGF.Builder.CreateLShr(V, ShiftCst);
+} 
+// We're truncating value in order to extract the higher-order
+// double, which we'll be using to extract the sign from.
 IntTy = llvm::IntegerType::get(C, Width);
 V = CGF.Builder.CreateTrunc(V, IntTy);
   }


Index: test/Analysis/builtin_signbit.cpp
===
--- test/Analysis/builtin_signbit.cpp
+++ test/Analysis/builtin_signbit.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang -target powerpc-linux-gnu -emit-llvm -S -O0 %s -o - | FileCheck %s --check-prefix=CHECK-BE --check-prefix=CHECK
+// RUN: %clang -target powerpc64-linux-gnu   -emit-llvm -S -O0 %s -o - | FileCheck %s --check-prefix=CHECK-BE --check-prefix=CHECK
+// RUN: %clang -target powerpc64le-linux-gnu -emit-llvm -S -O0 %s -o - | FileCheck %s --check-prefix=CHECK-LE --check-prefix=CHECK
+
+bool b;
+double d = -1.0;
+long double ld = -1.0L;
+void test_signbit()
+{
+  b = __builtin_signbit(1.0L);
+  // CHECK: i128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+
+  b = __builtin_signbit(ld);
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+
+  b = __builtin_signbitf(1.0);
+  // CHECK: store i8 0
+
+  b = __builtin_signbitf(d);
+  // CHECK: bitcast
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE-NOT: lshr
+
+  b = __builtin_signbitl(1.0L);
+  // CHECK: i128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+
+  b = __builtin_signbitl(ld);
+  // CHECK: bitcast
+  // CHECK: ppc_fp128
+  // CHECK-LE-NOT: lshr
+  // CHECK-BE: lshr
+}
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -238,10 +238,20 @@
   llvm::Type *IntTy = llvm::IntegerType::get(C, Width);
   V = CGF.Builder.CreateBitCast(V, IntTy);
   if (Ty->isPPC_FP128Ty()) {
-// The higher-order double comes first, and so we need to truncate the
-// pair to extract the overall sign. The order of the pair is the same
-// in both little- and big-Endian modes.
+// We want the sign bit of the higher-or

Re: [PATCH] D5767: Template Instantiation Observer + a few other templight-related changes

[Ábel Sinkovics via cfe-commits]

sabel83 added a comment.

Hi Manuel,

The patch is maintained in the Templight git repository 
(https://github.com/mikael-s-persson/templight/blob/master/templight_clang_patch.diff).
 I've written some tests for it (based on our discussion at CppCon), they are 
under review at the moment (see 
https://github.com/mikael-s-persson/templight/pull/25 for the details). Note 
that these are just some initial tests, not covering the entire patch.


http://reviews.llvm.org/D5767



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

Re: [PATCH] D12922: Add support for function attribute "notail"

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.


Comment at: lib/Sema/SemaDecl.cpp:5374
@@ +5373,3 @@
+
+  // Virtual functions cannot be marked as 'notail'.
+  if (auto *Attr = ND.getAttr())

aaron.ballman wrote:
> Is there a reason this is here instead of SemaDeclAttr.cpp? It seems like the 
> decl passed in with the attribute attached to it should already be known to 
> be virtual or not?
I initially did the check in SemaDeclAttr.cpp but found out that methods 
declared override are not known to be virtual (isVirtual returns false) when 
ProcessDeclAttributes is called:

class Base {
  // This method is fine.
  virtual int foo1();
};

class Derived1 : public Base {
  // isVirtual() is false for the declaration of this method.
  int foo1() override;
};

This is because ProcessDeclAttributes is called before CheckFunctionDeclaration 
(in line 7952, which calls Sema::AddOverriddenMethods) is called in 
Sema::ActOnFunctionDeclarator. Unless there is another way to check if a method 
is virtual, I don't think we can move the check to SemaDeclAttr.cpp?


http://reviews.llvm.org/D12922



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

Re: [PATCH] D13973: CFG: Delay creating Dtors for CompoundStmts which end in ReturnStmt

[Matthias Gehre via cfe-commits]

mgehre added a comment.

I see. Thinking about this, the same issue should also apply to a throw
inside the compound stmt. And it's not only about suppressing the dtors but
any CFG entries after return/throw. I'll prepare an updated patch.

Manuel Klimek  schrieb am So., 1. Nov. 2015 23:04:

> klimek added a reviewer: jordan_rose.

>  klimek added a comment.

> 

> +jordan for an opinion on whether  we want to fix the more general case.

> 

> My main problem is that while we're at it we need to fully understand the

>  change anyway, and if somebody runs into this later, they'll need a long

>  time debugging this surprising effect (like what happened to you here ;)

> 

> http://reviews.llvm.org/D13973



http://reviews.llvm.org/D13973



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