[PATCH] D60974: Clang IFSO driver action.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@plotfi Sorry I have to revert this patch. This can also cause problems in 
`-DBUILD_SHARED_LIBS=off` builds.  clangFrontend files cannot `#include 
"clang/Index/CodegenNameGenerator.h"`, I think you may have to move files 
around.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

r363649 - Revert D60974 "[clang-ifs] Clang Interface Stubs, first version."

[Fangrui Song via cfe-commits]

Author: maskray
Date: Mon Jun 17 22:52:39 2019
New Revision: 363649

URL: http://llvm.org/viewvc/llvm-project?rev=363649=rev
Log:
Revert D60974 "[clang-ifs] Clang Interface Stubs, first version."

This reverts commit rC363626.

clangIndex depends on clangFrontend. r363626 adds a dependency from
clangFrontend to clangIndex, which creates a circular dependency.

This is disallowed by -DBUILD_SHARED_LIBS=on builds:

CMake Error: The inter-target dependency graph contains the following 
strongly connected component (cycle):
  "clangFrontend" of type SHARED_LIBRARY
depends on "clangIndex" (weak)
  "clangIndex" of type SHARED_LIBRARY
depends on "clangFrontend" (weak)
At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies 
are allowed only among static libraries.

Note, the dependency on clangIndex cannot be removed because
libclangFrontend.so is linked with -Wl,-z,defs: a shared object must
have its full direct dependencies specified on the linker command line.

In -DBUILD_SHARED_LIBS=off builds, this appears to work when linking
`bin/clang-9`. However, it can cause trouble to downstream clang library
users. The llvm build system links libraries this way:

clang main_program_object_file ... lib/libclangIndex.a ...  
lib/libclangFrontend.a -o exe

libclangIndex.a etc are not wrapped in --start-group.

If the downstream application depends on libclangFrontend.a but not any
other clang libraries that depend on libclangIndex.a, this can cause undefined
reference errors when the linker is ld.bfd or gold.

The proper fix is to not include clangIndex files in clangFrontend.

Removed:
cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
cfe/trunk/test/InterfaceStubs/bad-format.cpp
cfe/trunk/test/InterfaceStubs/class-template-specialization.cpp
cfe/trunk/test/InterfaceStubs/externstatic.c
cfe/trunk/test/InterfaceStubs/function-template-specialization.cpp
cfe/trunk/test/InterfaceStubs/hidden-class-inheritance.cpp
cfe/trunk/test/InterfaceStubs/inline.c
cfe/trunk/test/InterfaceStubs/inline.h
cfe/trunk/test/InterfaceStubs/object.cpp
cfe/trunk/test/InterfaceStubs/template-namespace-function.cpp
cfe/trunk/test/InterfaceStubs/virtual.cpp
cfe/trunk/test/InterfaceStubs/visibility.cpp
cfe/trunk/test/InterfaceStubs/weak.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/Types.def
cfe/trunk/include/clang/Frontend/FrontendActions.h
cfe/trunk/include/clang/Frontend/FrontendOptions.h
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CMakeLists.txt
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=363649=363648=363649=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Mon Jun 17 
22:52:39 2019
@@ -220,8 +220,6 @@ def err_module_header_file_not_found :
 def err_module_header_file_invalid :
   Error<"unexpected module header file input '%0'">, DefaultFatal;
 
-def err_interface_stubs : Error<"clang-ifs (-emit-iterface-stubs): %0">;
-
 def err_test_module_file_extension_version : Error<
   "test module file extension '%0' has different version (%1.%2) than expected 
"
   "(%3.%4)">;

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=363649=363648=363649=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Jun 17 22:52:39 2019
@@ -623,9 +623,6 @@ def emit_ast : Flag<["-"], "emit-ast">,
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option]>, 
Group,
   HelpText<"Use the LLVM representation for assembler and object files">;
-def emit_iterface_stubs : Flag<["-"], "emit-interface-stubs">, 
Flags<[CC1Option]>, Group,
-  HelpText<"Generate Inteface Stub Files.">;
-def iterface_stub_version_EQ : JoinedOrSeparate<["-"], 
"interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
 def e : JoinedOrSeparate<["-"], "e">, Group;
 def fPIC : Flag<["-"], "fPIC">, Group;

Modified: cfe/trunk/include/clang/Driver/Types.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Types.def?rev=363649=363648=363649=diff
==
--- 

[PATCH] D60974: Clang IFSO driver action.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:12
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Index/CodegenNameGenerator.h"
+#include "clang/Sema/TemplateInstCallback.h"

> rL363646

This file references symbols from clangIndex (`#include 
"clang/Index/CodegenNameGenerator.h"`), so you can't remove the `clangIndex` 
dependency from `CMakeFiles.txt`, otherwise `-DBUILD_SHARED_LIBS=off` builds 
would also fail. 

```
# Pass -Wl,-z,defs. This makes sure all symbols are defined. Otherwise a DSO
# build might work on ELF but fail on MachO/COFF.
```

The built shared library must have all of its dependencies specified on the 
linker command line.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

I had to back out r363646. While it worked for me building locally with and 
without shared lib, it appears to have caused bots to break. I will take 
another look at fixing it in about 10 hours.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

r363648 - [NFC] Undoing r363646 to fix bots.

[Puyan Lotfi via cfe-commits]

Author: zer0
Date: Mon Jun 17 22:15:59 2019
New Revision: 363648

URL: http://llvm.org/viewvc/llvm-project?rev=363648=rev
Log:
[NFC] Undoing r363646 to fix bots.

-DBUILD_SHARED_LIBS=ON is still having problem caused by layering issues with
D60974. Locally there weren't problems building with shared libs on or off but
the bots appear to be acting up.



Modified:
cfe/trunk/lib/Frontend/CMakeLists.txt

Modified: cfe/trunk/lib/Frontend/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CMakeLists.txt?rev=363648=363647=363648=diff
==
--- cfe/trunk/lib/Frontend/CMakeLists.txt (original)
+++ cfe/trunk/lib/Frontend/CMakeLists.txt Mon Jun 17 22:15:59 2019
@@ -55,6 +55,7 @@ add_clang_library(clangFrontend
   clangAST
   clangBasic
   clangDriver
+  clangIndex
   clangEdit
   clangLex
   clangParse


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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

asb wrote:
> plotfi wrote:
> > MaskRay wrote:
> > > This is a layering issue. clangIndex depends on clangFrontend so 
> > > clangFrontend should not depend on clangIndex.
> > > 
> > > The circular dependency is allowed for static libraries but it breaks 
> > > `-DBUILD_SHARED_LIBS=on`:
> > > 
> > > ```
> > > CMake Error: The inter-target dependency graph contains the following 
> > > strongly connected component (cycle):
> > >   "clangFrontend" of type SHARED_LIBRARY
> > > depends on "clangIndex" (weak)
> > >   "clangIndex" of type SHARED_LIBRARY
> > > depends on "clangFrontend" (weak)
> > > At least one of these targets is not a STATIC_LIBRARY.  Cyclic 
> > > dependencies are allowed only among static libraries.
> > > ```
> > Is there a bot affected by this? I will take a look.
> Sadly there is no -DBUILD_SHARED_LIBS=True bot. This layering issue will 
> break builds for all downstream users using that option though.
Just checking (to see if there is a bot to verify with). I have a fix (removing 
clangIndex) that I think will build with or without -DBUILD_SHARED_LIBS=ON. 



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

plotfi wrote:
> asb wrote:
> > plotfi wrote:
> > > MaskRay wrote:
> > > > This is a layering issue. clangIndex depends on clangFrontend so 
> > > > clangFrontend should not depend on clangIndex.
> > > > 
> > > > The circular dependency is allowed for static libraries but it breaks 
> > > > `-DBUILD_SHARED_LIBS=on`:
> > > > 
> > > > ```
> > > > CMake Error: The inter-target dependency graph contains the following 
> > > > strongly connected component (cycle):
> > > >   "clangFrontend" of type SHARED_LIBRARY
> > > > depends on "clangIndex" (weak)
> > > >   "clangIndex" of type SHARED_LIBRARY
> > > > depends on "clangFrontend" (weak)
> > > > At least one of these targets is not a STATIC_LIBRARY.  Cyclic 
> > > > dependencies are allowed only among static libraries.
> > > > ```
> > > Is there a bot affected by this? I will take a look.
> > Sadly there is no -DBUILD_SHARED_LIBS=True bot. This layering issue will 
> > break builds for all downstream users using that option though.
> Just checking (to see if there is a bot to verify with). I have a fix 
> (removing clangIndex) that I think will build with or without 
> -DBUILD_SHARED_LIBS=ON. 
I've updated cfe/trunk/lib/Frontend/CMakeLists.txt in r363646


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

r363646 - [NFC] Fixing -DBUILD_SHARED_LIBS=ON problem caused by layering issue in D60974

[Puyan Lotfi via cfe-commits]

Author: zer0
Date: Mon Jun 17 21:40:03 2019
New Revision: 363646

URL: http://llvm.org/viewvc/llvm-project?rev=363646=rev
Log:
[NFC] Fixing -DBUILD_SHARED_LIBS=ON problem caused by layering issue in  D60974

Modified:
cfe/trunk/lib/Frontend/CMakeLists.txt

Modified: cfe/trunk/lib/Frontend/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CMakeLists.txt?rev=363646=363645=363646=diff
==
--- cfe/trunk/lib/Frontend/CMakeLists.txt (original)
+++ cfe/trunk/lib/Frontend/CMakeLists.txt Mon Jun 17 21:40:03 2019
@@ -55,7 +55,6 @@ add_clang_library(clangFrontend
   clangAST
   clangBasic
   clangDriver
-  clangIndex
   clangEdit
   clangLex
   clangParse


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

[PATCH] D60974: Clang IFSO driver action.

[Alex Bradbury via Phabricator via cfe-commits]

asb added inline comments.



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

plotfi wrote:
> MaskRay wrote:
> > This is a layering issue. clangIndex depends on clangFrontend so 
> > clangFrontend should not depend on clangIndex.
> > 
> > The circular dependency is allowed for static libraries but it breaks 
> > `-DBUILD_SHARED_LIBS=on`:
> > 
> > ```
> > CMake Error: The inter-target dependency graph contains the following 
> > strongly connected component (cycle):
> >   "clangFrontend" of type SHARED_LIBRARY
> > depends on "clangIndex" (weak)
> >   "clangIndex" of type SHARED_LIBRARY
> > depends on "clangFrontend" (weak)
> > At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies 
> > are allowed only among static libraries.
> > ```
> Is there a bot affected by this? I will take a look.
Sadly there is no -DBUILD_SHARED_LIBS=True bot. This layering issue will break 
builds for all downstream users using that option though.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D63473: Support -fclang-abi-compat=8.0 to keep old ABI behavior

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

Could you please add a test to ensure that Darwin defaults to the old behaviour?




Comment at: include/clang/Basic/LangOptions.h:142
+/// Attempt to be ABI-compatible with code generated by Clang 8.0.x
+/// (https://reviews.llvm.org/D59744). This causes __m64 to be passed in
+/// MMX register instead of integer register.

Referencing the SVN revision is better IMO as the infrastructure can change, 
but, the reference to the commit is useful as that can be mapped.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63473



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

[PATCH] D63473: Support -fclang-abi-compat=8.0 to keep old ABI behavior

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 created this revision.
wxiao3 added reviewers: annita.zhang, LuoYuanke, smaslov, craig.topper, 
hjl.tools, rnk, andreadb, gbedwell, rjmccall, krytarowski, mgorny, joerg, 
RKSimon, hans, thakis.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The System V i386 bug fix (https://reviews.llvm.org/D59744) makes it impossible
for 32-bit Linux Chromium to write an assembly function that works with both
 trunk clang and clang 8.0.0, which makes it difficult to update compilers
independent of changing the code (more details:
https://bugs.chromium.org/p/chromium/issues/detail?id=974542#c5).

This patch aims to provide a solution for such situation.


Repository:
  rC Clang

https://reviews.llvm.org/D63473

Files:
  docs/ReleaseNotes.rst
  include/clang/Basic/LangOptions.h
  lib/CodeGen/TargetInfo.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/x86_32-m64.c


Index: test/CodeGen/x86_32-m64.c
===
--- test/CodeGen/x86_32-m64.c
+++ test/CodeGen/x86_32-m64.c
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu 
yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft 
-emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,IAMCU
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-win32 -emit-llvm -o - %s | 
FileCheck %s --check-prefixes=CHECK,WIN32
+// RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-linux-gnu -target-cpu 
pentium4 -emit-llvm -fclang-abi-compat=8.0 -o - %s | FileCheck %s 
--check-prefixes=CHECK,OLDABI
 
 #include 
 __m64 m64;
@@ -24,6 +25,8 @@
 // WIN32-LABEL: define dso_local <1 x i64> @caller(i64 %__m1.coerce, i64 
%__m2.coerce)
 // WIN32: call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
 // WIN32: ret <1 x i64>
+// OLDABI: tail call void @callee(i64 %__m2.coerce, i64 %__m1.coerce)
+// OLDABI: ret <1 x i64>
   callee(__m2, __m1);
   return m64;
 }
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -3104,6 +3104,8 @@
 Opts.setClangABICompat(LangOptions::ClangABI::Ver6);
   else if (Major <= 7)
 Opts.setClangABICompat(LangOptions::ClangABI::Ver7);
+  else if (Major <= 8)
+Opts.setClangABICompat(LangOptions::ClangABI::Ver8);
 } else if (Ver != "latest") {
   Diags.Report(diag::err_drv_invalid_value)
   << A->getAsString(Args) << A->getValue();
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -1088,6 +1088,11 @@
   }
 
   bool isPassInMMXRegABI() const {
+// Clang <= 8.0 did not do this for compatiblity with old behavior.
+if (getContext().getLangOpts().getClangABICompat() <=
+LangOptions::ClangABI::Ver8)
+  return false;
+
 // The System V i386 psABI requires __m64 to be passed in MMX registers.
 // Clang historically had a bug where it failed to apply this rule, and
 // some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to maintain
Index: include/clang/Basic/LangOptions.h
===
--- include/clang/Basic/LangOptions.h
+++ include/clang/Basic/LangOptions.h
@@ -138,6 +138,11 @@
 /// rather than returning the required alignment.
 Ver7,
 
+/// Attempt to be ABI-compatible with code generated by Clang 8.0.x
+/// (https://reviews.llvm.org/D59744). This causes __m64 to be passed in
+/// MMX register instead of integer register.
+Ver8,
+
 /// Conform to the underlying platform's C and C++ ABIs as closely
 /// as we can.
 Latest
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -142,6 +142,14 @@
 ABI Changes in Clang
 
 
+- The System V i386 psABI requires __m64 to be passed in MMX registers.
+  Clang historically had a bug where it failed to apply this rule, and
+  some platforms (e.g. Darwin, PS4, and FreeBSD) have opted to maintain
+  compatibility with the old Clang behavior, so we only apply it on
+  platforms that have specifically requested it (currently just Linux and
+  NetBSD). You can switch back to old API behavior with flag:
+  -fclang-abi-compat=8.0.
+
 - ...
 
 OpenMP Support in Clang


Index: test/CodeGen/x86_32-m64.c
===
--- test/CodeGen/x86_32-m64.c
+++ test/CodeGen/x86_32-m64.c
@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-apple-darwin9 -target-cpu yonah -emit-llvm -o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
 // RUN: %clang_cc1 -w -O2 -fblocks -triple i386-pc-elfiamcu -mfloat-abi soft -emit-llvm -o - %s | FileCheck 

[PATCH] D59744: Fix i386 ABI "__m64" type bug

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 added a comment.

I have created a patch for you: https://reviews.llvm.org/D63473
Is it ok?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59744



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

RE: r363548 - Promote -fdebug-compilation-dir from a cc1 flag to clang and clang-cl driver flags

[via cfe-commits]

Hi Nico,

The test you added in cfe/trunk/test/Driver/clang_f_opts.c fails if a target 
does not use the integrated assembler. You can see what happens if you add 
"-fno-integrated-as" to the command line:

/home/dyung/src/upstream/363548-linux/bin/clang -### -fno-integrated-as 
-fdebug-compilation-dir . -x assembler clang_f_opts.c
clang version 9.0.0 (trunk 363548)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/dyung/src/upstream/363548-linux/bin
clang-9: warning: argument unused during compilation: '-fdebug-compilation-dir 
.' [-Wunused-command-line-argument]
 "/usr/bin/as" "--64" "-o" "/tmp/clang_f_opts-f2e231.o" "clang_f_opts.c"
 "/usr/bin/ld" "-z" "relro" "--hash-style=gnu" "--eh-frame-hdr" "-m" 
"elf_x86_64" "-dynamic-linker" "/lib64/ld-linux-x86-64.so.2" "-o" "a.out" 
"/usr/lib/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/crt1.o" 
"/usr/lib/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/crti.o" 
"/usr/lib/gcc/x86_64-linux-gnu/8/crtbegin.o" 
"-L/usr/lib/gcc/x86_64-linux-gnu/8" 
"-L/usr/lib/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu" 
"-L/lib/x86_64-linux-gnu" "-L/lib/../lib64" "-L/usr/lib/x86_64-linux-gnu" 
"-L/usr/lib/gcc/x86_64-linux-gnu/8/../../.." 
"-L/home/dyung/src/upstream/363548-linux/bin/../lib" "-L/lib" "-L/usr/lib" 
"/tmp/clang_f_opts-f2e231.o" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"-lc" "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed" 
"/usr/lib/gcc/x86_64-linux-gnu/8/crtend.o" 
"/usr/lib/gcc/x86_64-linux-gnu/8/../../../x86_64-linux-gnu/crtn.o"

Can you take a look to see if you can fix the test to work if a target does not 
use the integrated assembler?

Douglas Yung

-Original Message-
From: cfe-commits  On Behalf Of Nico Weber 
via cfe-commits
Sent: Monday, June 17, 2019 5:11
To: cfe-commits@lists.llvm.org
Subject: r363548 - Promote -fdebug-compilation-dir from a cc1 flag to clang and 
clang-cl driver flags

Author: nico
Date: Mon Jun 17 05:10:40 2019
New Revision: 363548

URL: http://llvm.org/viewvc/llvm-project?rev=363548=rev
Log:
Promote -fdebug-compilation-dir from a cc1 flag to clang and clang-cl driver 
flags

The flag is useful when wanting to create .o files that are independent from 
the absolute path to the build directory. -fdebug-prefix-map= can be used to 
the same effect, but it requires putting the absolute path to the build 
directory on the build command line, so it still requires the build command 
line to be dependent on the absolute path of the build directory. With this 
flag, "-fdebug-compilation-dir ." makes it so that both debug info and the 
compile command itself are independent of the absolute path of the build 
directory, which is good for build determinism (in the sense that the build is 
independent of which directory it happens in) and for caching compile results.
(The tradeoff is that the debugger needs explicit configuration to know the 
build directory. See also http://dwarfstd.org/ShowIssue.php?issue=171130.2)

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

Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/test/Driver/cl-options.c
cfe/trunk/test/Driver/clang_f_opts.c

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=363548=363547=363548=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Mon Jun 17 05:10:40 
+++ 2019
@@ -190,8 +190,6 @@ def default_function_attr : Separate<["-
   HelpText<"Apply given attribute to all functions">;  def dwarf_version_EQ : 
Joined<["-"], "dwarf-version=">;  def debugger_tuning_EQ : Joined<["-"], 
"debugger-tuning=">; -def fdebug_compilation_dir : Separate<["-"], 
"fdebug-compilation-dir">,
-  HelpText<"The compilation directory to embed in the debug info.">;  def 
dwarf_debug_flags : Separate<["-"], "dwarf-debug-flags">,
   HelpText<"The string to embed in the Dwarf debug flags record.">;  def 
record_command_line : Separate<["-"], "record-command-line">,

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=363548=363547=363548=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Jun 17 05:10:40 2019
@@ -713,11 +713,14 @@ def fauto_profile_accurate : Flag<["-"],
 Group, Alias;  def 
fno_auto_profile_accurate : Flag<["-"], "fno-auto-profile-accurate">,
 Group, Alias;
-def fdebug_info_for_profiling : Flag<["-"], "fdebug-info-for-profiling">, 
Group,
-Flags<[CC1Option]>,
+def fdebug_compilation_dir : Separate<["-"], "fdebug-compilation-dir">,
+Group, Flags<[CC1Option, 

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:22
+  StringRef Format;
+  std::set ParsedTemplates;
+

MaskRay wrote:
> Does `StringSet<>` work?
It probably could work. I will likely follow up with an NFC patch for this. 



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:36
+  };
+  using MangledSymbols = std::map;
+

MaskRay wrote:
> Are you relying on the ordered property of `std::map`?
No I am not. 



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:101
+  return true;
+if (Symbols.find(ND) != Symbols.end())
+  return true;

MaskRay wrote:
> `.count`
Nice. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

MaskRay wrote:
> This is a layering issue. clangIndex depends on clangFrontend so 
> clangFrontend should not depend on clangIndex.
> 
> The circular dependency is allowed for static libraries but it breaks 
> `-DBUILD_SHARED_LIBS=on`:
> 
> ```
> CMake Error: The inter-target dependency graph contains the following 
> strongly connected component (cycle):
>   "clangFrontend" of type SHARED_LIBRARY
> depends on "clangIndex" (weak)
>   "clangIndex" of type SHARED_LIBRARY
> depends on "clangFrontend" (weak)
> At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies 
> are allowed only among static libraries.
> ```
Is there a bot affected by this? I will take a look.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:22
+  StringRef Format;
+  std::set ParsedTemplates;
+

Does `StringSet<>` work?



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:36
+  };
+  using MangledSymbols = std::map;
+

Are you relying on the ordered property of `std::map`?



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:101
+  return true;
+if (Symbols.find(ND) != Symbols.end())
+  return true;

`.count`



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:103
+  return true;
+// - Currently have not figured out how to produce the names for 
FieldDecls.
+// - Do not want to produce symbols for function paremeters.

This should be a TODO



Comment at: cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:124
+
+  Symbols.insert(std::make_pair(
+  ND,

If you use a llvm container, `emplace` or `try_emplace`


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D60974: Clang IFSO driver action.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: cfe/trunk/lib/Frontend/CMakeLists.txt:58
   clangDriver
+  clangIndex
   clangEdit

This is a layering issue. clangIndex depends on clangFrontend so clangFrontend 
should not depend on clangIndex.

The circular dependency is allowed for static libraries but it breaks 
`-DBUILD_SHARED_LIBS=on`:

```
CMake Error: The inter-target dependency graph contains the following strongly 
connected component (cycle):
  "clangFrontend" of type SHARED_LIBRARY
depends on "clangIndex" (weak)
  "clangIndex" of type SHARED_LIBRARY
depends on "clangFrontend" (weak)
At least one of these targets is not a STATIC_LIBRARY.  Cyclic dependencies are 
allowed only among static libraries.
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974



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

[PATCH] D59744: Fix i386 ABI "__m64" type bug

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D59744#1547445 , @wxiao3 wrote:

> @hans
>
> Please make sure all Chromium for 32-bit Linux libraries are following System 
> V ABI (i.e., m64 is passed on mmx register). I suspect that there are some 
> hand written assembly code in your libraries which is not following the ABI.


That's likely true, but also not very helpful since the ABI implications here 
are pretty big (see comments on the chromium bug). It's also currently 
impossible to write an assembly function that works with both trunk clang and 
clang 8.0.0, which makes it difficult to update compilers independent of 
changing the code. clang generally tries to be abi-compatible with itself. This 
should probably support the existing fclang-abi-compat= flag at least (and have 
a release notes entry); possibly there should be a dedicated flag for this.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59744



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

[PATCH] D59744: Fix i386 ABI "__m64" type bug

[Wei Xiao via Phabricator via cfe-commits]

wxiao3 added a comment.

@hans

Please make sure all Chromium for 32-bit Linux libraries are following System V 
ABI (i.e., m64 is passed on mmx register). I suspect that there are some hand 
written assembly code in your libraries which is not following the ABI.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59744



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

RE: r363204 - [clang-scan-deps] initial outline of the tool that runs preprocessor to find

[via cfe-commits]

Hi Alex,

In the test you added here, the json file contains compile commands that 
includes "-c". Is that really necessary for the test to work properly?

The reason I ask is that if you run in a configuration which does not use the 
integrated assembler, the test fails with an error due to the fact that an 
external assembler must be invoked:

error: unable to handle compilation, expected exactly one compiler job in ' 
"clang" "-cc1" ...; "/usr/bin/as" "--64" "-I" "Inputs" "-o" "/dev/null"

Changing the "-c" to "-S" still shows the test passing when I run it locally, 
so if the "-c" is not really needed, can it be changed to "-S" to work 
correctly where the integrated assembler is not the default option?

Douglas Yung

-Original Message-
From: cfe-commits  On Behalf Of Alex Lorenz 
via cfe-commits
Sent: Wednesday, June 12, 2019 14:33
To: cfe-commits@lists.llvm.org
Subject: r363204 - [clang-scan-deps] initial outline of the tool that runs 
preprocessor to find

Author: arphaman
Date: Wed Jun 12 14:32:49 2019
New Revision: 363204

URL: http://llvm.org/viewvc/llvm-project?rev=363204=rev
Log:
[clang-scan-deps] initial outline of the tool that runs preprocessor to find 
dependencies over a JSON compilation database

This commit introduces an outline for the clang-scan-deps tool that will be 
used to implement fast dependency discovery phase using implicit modules for 
explicit module builds.

The initial version of the tool works by computing non-modular header 
dependencies for files in the compilation database without any optimizations 
(i.e. without source minimization from r362459).
The tool spawns a number of worker threads to run the clang compiler workers in 
parallel.

The immediate goal for clang-scan-deps is to create a ClangScanDeps library 
which will be used to build up this tool to use the source minimization and 
caching multi-threaded filesystem to implement the optimized non-incremental 
dependency scanning phase for a non-modular build. This will allow us to do 
benchmarks and comparisons for performance that the minimization and caching 
give us

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

Added:
cfe/trunk/test/ClangScanDeps/
cfe/trunk/test/ClangScanDeps/Inputs/
cfe/trunk/test/ClangScanDeps/Inputs/header.h
cfe/trunk/test/ClangScanDeps/Inputs/header2.h
cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
cfe/trunk/tools/clang-scan-deps/
cfe/trunk/tools/clang-scan-deps/CMakeLists.txt
cfe/trunk/tools/clang-scan-deps/ClangScanDeps.cpp
Modified:
cfe/trunk/test/CMakeLists.txt
cfe/trunk/tools/CMakeLists.txt

Modified: cfe/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=363204=363203=363204=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Wed Jun 12 14:32:49 2019
@@ -57,6 +57,7 @@ list(APPEND CLANG_TEST_DEPS
   clang-rename
   clang-refactor
   clang-diff
+  clang-scan-deps
   diagtool
   hmaptool
   )

Added: cfe/trunk/test/ClangScanDeps/Inputs/header.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/header.h?rev=363204=auto
==
--- cfe/trunk/test/ClangScanDeps/Inputs/header.h (added)
+++ cfe/trunk/test/ClangScanDeps/Inputs/header.h Wed Jun 12 14:32:49 
+++ 2019
@@ -0,0 +1,3 @@
+#ifdef INCLUDE_HEADER2
+#include "header2.h"
+#endif

Added: cfe/trunk/test/ClangScanDeps/Inputs/header2.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/header2.h?rev=363204=auto
==
--- cfe/trunk/test/ClangScanDeps/Inputs/header2.h (added)
+++ cfe/trunk/test/ClangScanDeps/Inputs/header2.h Wed Jun 12 14:32:49 
+++ 2019
@@ -0,0 +1 @@
+// header 2.

Added: cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json?rev=363204=auto
==
--- cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json (added)
+++ cfe/trunk/test/ClangScanDeps/Inputs/regular_cdb.json Wed Jun 12 
+++ 14:32:49 2019
@@ -0,0 +1,12 @@
+[
+{
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -MD -MF 
+DIR/regular_cdb.d",
+  "file": "DIR/regular_cdb.cpp"
+},
+{
+  "directory": "DIR",
+  "command": "clang -c DIR/regular_cdb.cpp -IInputs -D INCLUDE_HEADER2 
+-MD -MF DIR/regular_cdb2.d",
+  "file": "DIR/regular_cdb.cpp"
+}
+]

Added: cfe/trunk/test/ClangScanDeps/regular_cdb.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ClangScanDeps/regular_cdb.cpp?rev=363204=auto
==
--- 

[PATCH] D57086: Ignore trailing NullStmts in StmtExprs for GCC compatibility

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

rsmith added a comment.

In D57086#1546386 , @aaron.ballman 
wrote:

> In D57086#1535873 , @domdom wrote:
>
> > Something I should ask, it seems like GCC only ignores the NullStmts at the 
> > end if it's in C mode. Should clang match this behaviour exactly?
>
>
> I can't think of a reason that this should only happen in C mode, can you 
> @rsmith?


No, I can't think of such a reason either. When we've seen such weirdnesses 
before (eg, `break` or `continue` in a statement expression in a `for` loop 
increment expression) we generally try to pick a behavior that's consistent 
across languages, and warn on the cases where we give a different behavior from 
GCC as a result.


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

https://reviews.llvm.org/D57086



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

[PATCH] D62635: Add enums as global variables in the IR metadata.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

They should all be there, but emitting the unused enums makes the binary sizes 
larger. (I think around 6% increase? I forget the size difference for only 
emitting used enums)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62635



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

[PATCH] D62635: Add enums as global variables in the IR metadata.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D62635#1547352 , @akhuang wrote:

> I think the main issue was keeping track of which enums are used?


The used enums would still end up in the 'enums' list for the DICompileUnit, 
right? (If CodeView needs more enums in the list than DWARF does - yeah, that's 
probably a necessary frontend change - to put the extra needed enums in the 
enums list)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62635



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

[PATCH] D62635: Add enums as global variables in the IR metadata.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

I think the main issue was keeping track of which enums are used?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62635



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

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso accepted this revision.
Charusso marked an inline comment as done.
Charusso added a comment.

Thanks for the main development! Could I start to reduce the size with my 
SVG-magic? What do you think about make it more SVG-based on the styling side?




Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:375
+def dump_binding(f, b, is_added=None):
+self._dump('%s'
+   'S%s'

Good catch!


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

https://reviews.llvm.org/D62761



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

[PATCH] D63161: Devirtualize destructor of final class.

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

rsmith added inline comments.



Comment at: lib/CodeGen/CGExprCXX.cpp:1887
+Dtor = DevirtualizedDtor;
+Ptr = CGF.EmitPointerWithAlignment(Inner);
+  } else {

yamauchi wrote:
> rsmith wrote:
> > In this case we'll emit the inner expression (including its side-effects) 
> > twice.
> > 
> > The simplest thing to do would be to just drop this `else if` case for now 
> > and add a FIXME.
> I'd go with that. I assume there isn't a simple way to adjust the this 
> pointer of a base class given a derived class? Sort of like 
> CodeGenFunction::GetAddressOfDerivedClass?
> 
I don't think we have anything quite like that. This won't be possible in the 
general case: there could be more than one base class of the current type 
within the derived class, so you'd need to take into account the path that the 
base expression took to reach the inner expression with the known dynamic type. 
(We could opportunistically do this in the common case where the derived class 
has only one base class of the base type, and it's a non-virtual base, but I 
don't think we have any existing code to do that either.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D63161



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

[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

[Csaba Dabis via Phabricator via cfe-commits]

Charusso marked 2 inline comments as done.
Charusso added a comment.

Thanks for the review!




Comment at: clang/test/Analysis/dump_egraph.c:3
 // RUN: cat %t.dot | FileCheck %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot 
-trim-egraph %s
 // REQUIRES: asserts

NoQ wrote:
> Charusso wrote:
> > This did nothing.
> Let's fix it by adding another `// RUN: cat %t.dot | FileCheck %s` after this 
> line instead, so that both trimmed graph and untrimmed graph gets tested.
I am not sure why `Report.getErrorNode() == N` is not enough, but now it passes 
well and satisfies the problem when we reported something on hidden nodes.


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

https://reviews.llvm.org/D63436



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

[PATCH] D63462: [analyzer] JsonSupport: Escape escapes

[Csaba Dabis via Phabricator via cfe-commits]

Charusso created this revision.
Charusso added a reviewer: NoQ.
Charusso added a project: clang.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.

-


Repository:
  rC Clang

https://reviews.llvm.org/D63462

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


Index: clang/include/clang/Basic/JsonSupport.h
===
--- clang/include/clang/Basic/JsonSupport.h
+++ clang/include/clang/Basic/JsonSupport.h
@@ -31,7 +31,26 @@
   std::string Str = RawSR.trim().str();
   size_t Pos = 0;
 
+  // Escape backslashes.
+  while (true) {
+Pos = Str.find('\\', Pos);
+if (Pos == std::string::npos)
+  break;
+
+// Prevent bad conversions.
+size_t TempPos = (Pos != 0) ? Pos - 1 : 0;
+
+// See whether the current backslash is not escaped.
+if (TempPos != Str.find("", Pos)) {
+  Str.insert(Pos, "\\");
+  ++Pos; // As we insert the backslash move plus one.
+}
+
+++Pos;
+  }
+
   // Escape double quotes.
+  Pos = 0;
   while (true) {
 Pos = Str.find('\"', Pos);
 if (Pos == std::string::npos)
@@ -40,8 +59,8 @@
 // Prevent bad conversions.
 size_t TempPos = (Pos != 0) ? Pos - 1 : 0;
 
-// See whether the current double quote is escaped.
-if (TempPos != Str.find("\\\"", TempPos)) {
+// See whether the current double quote is not escaped.
+if (TempPos != Str.find("\\\"", Pos)) {
   Str.insert(Pos, "\\");
   ++Pos; // As we insert the escape-character move plus one.
 }


Index: clang/include/clang/Basic/JsonSupport.h
===
--- clang/include/clang/Basic/JsonSupport.h
+++ clang/include/clang/Basic/JsonSupport.h
@@ -31,7 +31,26 @@
   std::string Str = RawSR.trim().str();
   size_t Pos = 0;
 
+  // Escape backslashes.
+  while (true) {
+Pos = Str.find('\\', Pos);
+if (Pos == std::string::npos)
+  break;
+
+// Prevent bad conversions.
+size_t TempPos = (Pos != 0) ? Pos - 1 : 0;
+
+// See whether the current backslash is not escaped.
+if (TempPos != Str.find("", Pos)) {
+  Str.insert(Pos, "\\");
+  ++Pos; // As we insert the backslash move plus one.
+}
+
+++Pos;
+  }
+
   // Escape double quotes.
+  Pos = 0;
   while (true) {
 Pos = Str.find('\"', Pos);
 if (Pos == std::string::npos)
@@ -40,8 +59,8 @@
 // Prevent bad conversions.
 size_t TempPos = (Pos != 0) ? Pos - 1 : 0;
 
-// See whether the current double quote is escaped.
-if (TempPos != Str.find("\\\"", TempPos)) {
+// See whether the current double quote is not escaped.
+if (TempPos != Str.find("\\\"", Pos)) {
   Str.insert(Pos, "\\");
   ++Pos; // As we insert the escape-character move plus one.
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63438: [analyzer] print() JSONify: ProgramPoint revision

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 205209.
Charusso added a comment.

- Added a test case.


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

https://reviews.llvm.org/D63438

Files:
  clang/lib/Analysis/ProgramPoint.cpp
  clang/test/Analysis/dump_egraph.c


Index: clang/test/Analysis/dump_egraph.c
===
--- clang/test/Analysis/dump_egraph.c
+++ clang/test/Analysis/dump_egraph.c
@@ -22,3 +22,5 @@
 
 // CHECK: \"has_report\": true
 
+// CHECK: \"pretty\": \"*x\", \"location\": \{ \"line\": 16, \"column\": 10, 
\"file\": \"{{(.+)}}dump_egraph.c\" \}
+
Index: clang/lib/Analysis/ProgramPoint.cpp
===
--- clang/lib/Analysis/ProgramPoint.cpp
+++ clang/lib/Analysis/ProgramPoint.cpp
@@ -55,7 +55,8 @@
   }
 
   Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
-  << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
+  << ", \"column\": " << SM.getExpansionColumnNumber(Loc)
+  << ", \"file\": \"" << SM.getFilename(Loc) << "\" }";
 }
 
 void ProgramPoint::printJson(llvm::raw_ostream , const char *NL) const {


Index: clang/test/Analysis/dump_egraph.c
===
--- clang/test/Analysis/dump_egraph.c
+++ clang/test/Analysis/dump_egraph.c
@@ -22,3 +22,5 @@
 
 // CHECK: \"has_report\": true
 
+// CHECK: \"pretty\": \"*x\", \"location\": \{ \"line\": 16, \"column\": 10, \"file\": \"{{(.+)}}dump_egraph.c\" \}
+
Index: clang/lib/Analysis/ProgramPoint.cpp
===
--- clang/lib/Analysis/ProgramPoint.cpp
+++ clang/lib/Analysis/ProgramPoint.cpp
@@ -55,7 +55,8 @@
   }
 
   Out << "{ \"line\": " << SM.getExpansionLineNumber(Loc)
-  << ", \"column\": " << SM.getExpansionColumnNumber(Loc) << " }";
+  << ", \"column\": " << SM.getExpansionColumnNumber(Loc)
+  << ", \"file\": \"" << SM.getFilename(Loc) << "\" }";
 }
 
 void ProgramPoint::printJson(llvm::raw_ostream , const char *NL) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 205208.
Charusso edited the summary of this revision.

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

https://reviews.llvm.org/D63436

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/dump_egraph.c


Index: clang/test/Analysis/dump_egraph.c
===
--- clang/test/Analysis/dump_egraph.c
+++ clang/test/Analysis/dump_egraph.c
@@ -1,6 +1,12 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot 
%s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-dump-egraph=%t.dot %s
 // RUN: cat %t.dot | FileCheck %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot 
-trim-egraph %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-dump-egraph=%t.dot \
+// RUN:  -trim-egraph %s
+// RUN: cat %t.dot | FileCheck %s
+
 // REQUIRES: asserts
 
 int getJ();
@@ -10,8 +16,6 @@
   return *x + *y;
 }
 
-// CHECK: digraph "Exploded Graph" {
-
 // CHECK: \"program_points\": [\l\{ \"kind\": 
\"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": 
null, \"tag\": null \}\l],\l\"program_state\": null
 
 // CHECK: \"program_points\": [\l\{ \"kind\": 
\"BlockEntrance\", \"block_id\": 1
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3012,7 +3012,7 @@
 
 for (const auto  : EQClasses) {
   for (const BugReport  : EQ) {
-if (Report.getErrorNode() == N)
+if (Report.getErrorNode()->getState() == N->getState())
   return true;
   }
 }
@@ -3112,11 +3112,7 @@
   Indent(Out, Space, IsDot) << "\"program_state\": null";
 }
 
-Out << "\\l}";
-if (!N->succ_empty())
-  Out << ',';
-Out << "\\l";
-
+Out << "\\l}\\l";
 return Out.str();
   }
 };


Index: clang/test/Analysis/dump_egraph.c
===
--- clang/test/Analysis/dump_egraph.c
+++ clang/test/Analysis/dump_egraph.c
@@ -1,6 +1,12 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-dump-egraph=%t.dot %s
 // RUN: cat %t.dot | FileCheck %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot -trim-egraph %s
+
+// RUN: %clang_analyze_cc1 -analyzer-checker=core \
+// RUN:  -analyzer-dump-egraph=%t.dot \
+// RUN:  -trim-egraph %s
+// RUN: cat %t.dot | FileCheck %s
+
 // REQUIRES: asserts
 
 int getJ();
@@ -10,8 +16,6 @@
   return *x + *y;
 }
 
-// CHECK: digraph "Exploded Graph" {
-
 // CHECK: \"program_points\": [\l\{ \"kind\": \"Edge\", \"src_id\": 2, \"dst_id\": 1, \"terminator\": null, \"term_kind\": null, \"tag\": null \}\l],\l\"program_state\": null
 
 // CHECK: \"program_points\": [\l\{ \"kind\": \"BlockEntrance\", \"block_id\": 1
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3012,7 +3012,7 @@
 
 for (const auto  : EQClasses) {
   for (const BugReport  : EQ) {
-if (Report.getErrorNode() == N)
+if (Report.getErrorNode()->getState() == N->getState())
   return true;
   }
 }
@@ -3112,11 +3112,7 @@
   Indent(Out, Space, IsDot) << "\"program_state\": null";
 }
 
-Out << "\\l}";
-if (!N->succ_empty())
-  Out << ',';
-Out << "\\l";
-
+Out << "\\l}\\l";
 return Out.str();
   }
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62635: Add enums as global variables in the IR metadata.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Is it practical/possible to do this in LLVM rather than in Clang? I'd have 
thought it'd be best to keep the IR metadata as output-format agnostic as 
practical/possible to reduce divergent codepaths, etc?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62635



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

r363627 - [Remarks][Driver] Use the specified format in the remarks file extension

[Francis Visoiu Mistrih via cfe-commits]

Author: thegameg
Date: Mon Jun 17 15:49:38 2019
New Revision: 363627

URL: http://llvm.org/viewvc/llvm-project?rev=363627=rev
Log:
[Remarks][Driver] Use the specified format in the remarks file extension

By default, use `.opt.yaml`, but when a format is specified with
`-fsave-optimization-record=`, use `.opt.`.

Modified:
cfe/trunk/docs/UsersManual.rst
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/darwin-ld.c
cfe/trunk/test/Driver/opt-record.c

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=363627=363626=363627=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Mon Jun 17 15:49:38 2019
@@ -351,7 +351,9 @@ output format of the diagnostics that it
 
If this option is not used, optimization records are output to a file named
after the primary file being compiled. If that's "foo.c", for example,
-   optimization records are output to "foo.opt.yaml".
+   optimization records are output to "foo.opt.yaml". If a specific
+   serialization format is specified, the file will be named
+   "foo.opt.".
 
 .. _opt_foptimization-record-passes:
 

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=363627=363626=363627=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Jun 17 15:49:38 2019
@@ -5132,9 +5132,17 @@ void Clang::ConstructJob(Compilation ,
 }
   }
 
-  llvm::sys::path::replace_extension(F, "opt.yaml");
+  std::string Extension = "opt.";
+  if (const Arg *A =
+  Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+Extension += A->getValue();
+  else
+Extension += "yaml";
+
+  llvm::sys::path::replace_extension(F, Extension);
   CmdArgs.push_back(Args.MakeArgString(F));
 }
+
 if (const Arg *A =
 Args.getLastArg(options::OPT_foptimization_record_passes_EQ)) {
   CmdArgs.push_back("-opt-record-passes");

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=363627=363626=363627=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Mon Jun 17 15:49:38 2019
@@ -470,7 +470,13 @@ void darwin::Linker::ConstructJob(Compil
 
 SmallString<128> F;
 F = Output.getFilename();
-F += ".opt.yaml";
+F += ".opt.";
+if (const Arg *A =
+Args.getLastArg(options::OPT_fsave_optimization_record_EQ))
+  F += A->getValue();
+else
+  F += "yaml";
+
 CmdArgs.push_back(Args.MakeArgString(F));
 
 if (getLastProfileUseArg(Args)) {

Modified: cfe/trunk/test/Driver/darwin-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld.c?rev=363627=363626=363627=diff
==
--- cfe/trunk/test/Driver/darwin-ld.c (original)
+++ cfe/trunk/test/Driver/darwin-ld.c Mon Jun 17 15:49:38 2019
@@ -333,7 +333,7 @@
 //
 // RUN: %clang -target x86_64-apple-darwin12 %t.o 
-fsave-optimization-record=some-format -### -o foo/bar.out 2> %t.log
 // RUN: FileCheck -check-prefix=PASS_REMARKS_WITH_FORMAT %s < %t.log
-// PASS_REMARKS_WITH_FORMAT: "-mllvm" "-lto-pass-remarks-output" "-mllvm" 
"foo/bar.out.opt.yaml" "-mllvm" "-lto-pass-remarks-format=some-format"
+// PASS_REMARKS_WITH_FORMAT: "-mllvm" "-lto-pass-remarks-output" "-mllvm" 
"foo/bar.out.opt.some-format" "-mllvm" "-lto-pass-remarks-format=some-format"
 
 // RUN: %clang -target x86_64-apple-ios6.0 -miphoneos-version-min=6.0 
-fprofile-instr-generate -### %t.o 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_PROFILE_FIRST %s < %t.log

Modified: cfe/trunk/test/Driver/opt-record.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/opt-record.c?rev=363627=363626=363627=diff
==
--- cfe/trunk/test/Driver/opt-record.c (original)
+++ cfe/trunk/test/Driver/opt-record.c Mon Jun 17 15:49:38 2019
@@ -37,6 +37,7 @@
 // CHECK-FOPT-DISABLE-PASSES-NOT: "-fno-save-optimization-record"
 
 // CHECK-EQ-FORMAT: "-cc1"
+// CHECK-EQ-FORMAT: "-opt-record-file" "FOO.opt.some-format"
 // CHECK-EQ-FORMAT: "-opt-record-format" "some-format"
 
 // CHECK-FOPT-DISABLE-FORMAT-NOT: "-fno-save-optimization-record"


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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

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



Comment at: test/SemaCXX/warn-xor-as-pow.cpp:13
+res = a ^ b;
+res = 2 ^ 0;
+res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean 
'1<<1' (2)?}}

jfb wrote:
> `2 ^ 0` seems like it would be a bug too?
Small question..

Instead of 1<<0 for this case, I emit fixit "1", seems ok for you?


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

https://reviews.llvm.org/D63423



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363626: [clang-ifs] Clang Interface Stubs, first version. 
(authored by zer0, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60974?vs=205191=205203#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60974

Files:
  cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Driver/Types.def
  cfe/trunk/include/clang/Frontend/FrontendActions.h
  cfe/trunk/include/clang/Frontend/FrontendOptions.h
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CMakeLists.txt
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  cfe/trunk/test/InterfaceStubs/bad-format.cpp
  cfe/trunk/test/InterfaceStubs/class-template-specialization.cpp
  cfe/trunk/test/InterfaceStubs/externstatic.c
  cfe/trunk/test/InterfaceStubs/function-template-specialization.cpp
  cfe/trunk/test/InterfaceStubs/hidden-class-inheritance.cpp
  cfe/trunk/test/InterfaceStubs/inline.c
  cfe/trunk/test/InterfaceStubs/inline.h
  cfe/trunk/test/InterfaceStubs/object.cpp
  cfe/trunk/test/InterfaceStubs/template-namespace-function.cpp
  cfe/trunk/test/InterfaceStubs/virtual.cpp
  cfe/trunk/test/InterfaceStubs/visibility.cpp
  cfe/trunk/test/InterfaceStubs/weak.cpp

Index: cfe/trunk/include/clang/Driver/Options.td
===
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -623,6 +623,9 @@
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option]>, Group,
   HelpText<"Use the LLVM representation for assembler and object files">;
+def emit_iterface_stubs : Flag<["-"], "emit-interface-stubs">, Flags<[CC1Option]>, Group,
+  HelpText<"Generate Inteface Stub Files.">;
+def iterface_stub_version_EQ : JoinedOrSeparate<["-"], "interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
 def e : JoinedOrSeparate<["-"], "e">, Group;
 def fPIC : Flag<["-"], "fPIC">, Group;
Index: cfe/trunk/include/clang/Driver/Types.def
===
--- cfe/trunk/include/clang/Driver/Types.def
+++ cfe/trunk/include/clang/Driver/Types.def
@@ -88,6 +88,7 @@
 
 // Misc.
 TYPE("ast",  AST,  INVALID, "ast",   "u")
+TYPE("ifs",  IFS,  INVALID, "ifs",   "u")
 TYPE("pcm",  ModuleFile,   INVALID, "pcm",   "u")
 TYPE("plist",Plist,INVALID, "plist", "")
 TYPE("rewritten-objc",   RewrittenObjC,INVALID, "cpp",   "")
Index: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -220,6 +220,8 @@
 def err_module_header_file_invalid :
   Error<"unexpected module header file input '%0'">, DefaultFatal;
 
+def err_interface_stubs : Error<"clang-ifs (-emit-iterface-stubs): %0">;
+
 def err_test_module_file_extension_version : Error<
   "test module file extension '%0' has different version (%1.%2) than expected "
   "(%3.%4)">;
Index: cfe/trunk/include/clang/Frontend/FrontendOptions.h
===
--- cfe/trunk/include/clang/Frontend/FrontendOptions.h
+++ cfe/trunk/include/clang/Frontend/FrontendOptions.h
@@ -88,6 +88,10 @@
   /// Generate pre-compiled header.
   GeneratePCH,
 
+  /// Generate Interface Stub Files.
+  GenerateInterfaceYAMLExpV1,
+  GenerateInterfaceTBEExpV1,
+
   /// Only execute frontend initialization.
   InitOnly,
 
Index: cfe/trunk/include/clang/Frontend/FrontendActions.h
===
--- cfe/trunk/include/clang/Frontend/FrontendActions.h
+++ cfe/trunk/include/clang/Frontend/FrontendActions.h
@@ -119,6 +119,26 @@
   bool hasASTFileSupport() const override { return false; }
 };
 
+class GenerateInterfaceStubAction : public ASTFrontendAction {
+protected:
+  TranslationUnitKind getTranslationUnitKind() override { return TU_Module; }
+
+  bool hasASTFileSupport() const override { return false; }
+};
+
+// Support different interface stub formats this way:
+class GenerateInterfaceYAMLExpV1Action : public GenerateInterfaceStubAction {
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef InFile) override;
+};
+
+class GenerateInterfaceTBEExpV1Action : public GenerateInterfaceStubAction {

r363626 - [clang-ifs] Clang Interface Stubs, first version.

[Puyan Lotfi via cfe-commits]

Author: zer0
Date: Mon Jun 17 15:46:54 2019
New Revision: 363626

URL: http://llvm.org/viewvc/llvm-project?rev=363626=rev
Log:
[clang-ifs] Clang Interface Stubs, first version.

Clang interface stubs (previously referred to as clang-ifsos) is a new frontend
action in clang that allows the generation of stub files that contain mangled
name info that can be used to produce a stub library. These stub libraries can
be useful for breaking up build dependencies and controlling access to a
library's internal symbols. Generation of these stubs can be invoked by:

clang -fvisibility= -emit-interface-stubs \
-interface-stub-version=

Notice that -fvisibility (along with use of visibility attributes) can be used
to control what symbols get generated. Currently the interface format is
experimental but there are a wide range of possibilities here.

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



Added:
cfe/trunk/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
cfe/trunk/test/InterfaceStubs/
cfe/trunk/test/InterfaceStubs/bad-format.cpp
cfe/trunk/test/InterfaceStubs/class-template-specialization.cpp
cfe/trunk/test/InterfaceStubs/externstatic.c
cfe/trunk/test/InterfaceStubs/function-template-specialization.cpp
cfe/trunk/test/InterfaceStubs/hidden-class-inheritance.cpp
cfe/trunk/test/InterfaceStubs/inline.c
cfe/trunk/test/InterfaceStubs/inline.h
cfe/trunk/test/InterfaceStubs/object.cpp
cfe/trunk/test/InterfaceStubs/template-namespace-function.cpp
cfe/trunk/test/InterfaceStubs/virtual.cpp
cfe/trunk/test/InterfaceStubs/visibility.cpp
cfe/trunk/test/InterfaceStubs/weak.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/Types.def
cfe/trunk/include/clang/Frontend/FrontendActions.h
cfe/trunk/include/clang/Frontend/FrontendOptions.h
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CMakeLists.txt
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/FrontendTool/ExecuteCompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=363626=363625=363626=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Mon Jun 17 
15:46:54 2019
@@ -220,6 +220,8 @@ def err_module_header_file_not_found :
 def err_module_header_file_invalid :
   Error<"unexpected module header file input '%0'">, DefaultFatal;
 
+def err_interface_stubs : Error<"clang-ifs (-emit-iterface-stubs): %0">;
+
 def err_test_module_file_extension_version : Error<
   "test module file extension '%0' has different version (%1.%2) than expected 
"
   "(%3.%4)">;

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=363626=363625=363626=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Jun 17 15:46:54 2019
@@ -623,6 +623,9 @@ def emit_ast : Flag<["-"], "emit-ast">,
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option]>, 
Group,
   HelpText<"Use the LLVM representation for assembler and object files">;
+def emit_iterface_stubs : Flag<["-"], "emit-interface-stubs">, 
Flags<[CC1Option]>, Group,
+  HelpText<"Generate Inteface Stub Files.">;
+def iterface_stub_version_EQ : JoinedOrSeparate<["-"], 
"interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
 def e : JoinedOrSeparate<["-"], "e">, Group;
 def fPIC : Flag<["-"], "fPIC">, Group;

Modified: cfe/trunk/include/clang/Driver/Types.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Types.def?rev=363626=363625=363626=diff
==
--- cfe/trunk/include/clang/Driver/Types.def (original)
+++ cfe/trunk/include/clang/Driver/Types.def Mon Jun 17 15:46:54 2019
@@ -88,6 +88,7 @@ TYPE("lto-bc",   LTO_BC,
 
 // Misc.
 TYPE("ast",  AST,  INVALID, "ast",   "u")
+TYPE("ifs",  IFS,  INVALID, "ifs",   "u")
 TYPE("pcm",  ModuleFile,   INVALID, "pcm",   "u")
 TYPE("plist",Plist,INVALID, "plist", "")
 TYPE("rewritten-objc",   RewrittenObjC,INVALID, "cpp",   "")

Modified: cfe/trunk/include/clang/Frontend/FrontendActions.h
URL: 

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

xbolva00 updated this revision to Diff 205202.
xbolva00 added a comment.

Removed unused diagnostic message.


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define FOOBAR(x, y) (x * y)
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+#define EPSILON 10 ^ -300
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ -1;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+  res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0, maybe you mean '1<<2' (4)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<2"
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1<<16"
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
+  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<= width of type}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL<<32"
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '0x2 ^ 64' to silence this warning}}
+
+  res = 10 ^ 0; // expected-warning {{result of '10 ^ 0' is 10, maybe you mean '1e0'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e0"
+  // expected-note@-2 {{replace expression with '0xA ^ 0' to silence this warning}}
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '1e1'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e1"
+  // expected-note@-2 {{replace expression with '0xA ^ 1' to silence this warning}}
+  res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8, maybe you mean '1e2'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e2"
+  // expected-note@-2 {{replace expression with '0xA ^ 2' to silence this warning}}
+  res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14, maybe you mean '1e4'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e4"
+  // expected-note@-2 {{replace expression with '0xA ^ 4' to silence this warning}}
+  res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '1e10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1e10"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '1e10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"1e10"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = 10 ^ 100; // expected-warning {{result of '10 ^ 100' is 110, maybe you mean '1e100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"1e100"
+  // expected-note@-2 {{replace expression with '0xA ^ 100' to silence this warning}}
+  res = 0xA ^ 10;
+#if 

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

xbolva00 added a comment.

In D63423#1547244 , @regehr wrote:

> In D63423#1547216 , @xbolva00 wrote:
>
> > The only remaining question is, as your said, whether or not to diagnose 
> > xors in macros. @regerh @jfb ?
>
>
> IMO it's better to not warn about xors in macros-- I like the current 
> tradeoffs.


Thank you for opinion.

I think this patch is (almost) done. :)


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

https://reviews.llvm.org/D63423



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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

xbolva00 updated this revision to Diff 205198.
xbolva00 added a comment.

Addressed review notes.


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,98 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define FOOBAR(x, y) (x * y)
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+#define EPSILON 10 ^ -300
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ -1;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+  res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0, maybe you mean '1<<2' (4)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<2"
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1<<16"
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
+  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<= width of type}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL<<32"
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '0x2 ^ 64' to silence this warning}}
+
+  res = 10 ^ 0; // expected-warning {{result of '10 ^ 0' is 10, maybe you mean '1e0'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e0"
+  // expected-note@-2 {{replace expression with '0xA ^ 0' to silence this warning}}
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '1e1'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e1"
+  // expected-note@-2 {{replace expression with '0xA ^ 1' to silence this warning}}
+  res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8, maybe you mean '1e2'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e2"
+  // expected-note@-2 {{replace expression with '0xA ^ 2' to silence this warning}}
+  res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14, maybe you mean '1e4'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1e4"
+  // expected-note@-2 {{replace expression with '0xA ^ 4' to silence this warning}}
+  res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '1e10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1e10"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '1e10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"1e10"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = 10 ^ 100; // expected-warning {{result of '10 ^ 100' is 110, maybe you mean '1e100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"1e100"
+  // expected-note@-2 {{replace expression with '0xA ^ 100' to silence this warning}}
+  res = 0xA ^ 10;
+#if defined(KW_XOR)
+  

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[John Regehr via Phabricator via cfe-commits]

regehr added a comment.

In D63423#1547216 , @xbolva00 wrote:

> The only remaining question is, as your said, whether or not to diagnose xors 
> in macros. @regerh @jfb ?


IMO it's better to not warn about xors in macros-- I like the current tradeoffs.


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

https://reviews.llvm.org/D63423



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

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1640-1642
+  CFGBlock *OriginB = GetRelevantBlock(Origin);
+  if (!OriginB || !NB)
+return nullptr;

Szelethus wrote:
> NoQ wrote:
> > `// TODO: This can be cached.`
> Like, caching the `CFGBlock`s for given `ExplodedNode`s?
I mean, on this line it's the same block every time (per instance of a visitor).


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

https://reviews.llvm.org/D62883



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

[PATCH] D63161: Devirtualize destructor of final class.

[Hiroshi Yamauchi via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGExprCXX.cpp:1887
+Dtor = DevirtualizedDtor;
+Ptr = CGF.EmitPointerWithAlignment(Inner);
+  } else {

rsmith wrote:
> In this case we'll emit the inner expression (including its side-effects) 
> twice.
> 
> The simplest thing to do would be to just drop this `else if` case for now 
> and add a FIXME.
I'd go with that. I assume there isn't a simple way to adjust the this pointer 
of a base class given a derived class? Sort of like 
CodeGenFunction::GetAddressOfDerivedClass?



Repository:
  rC Clang

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

https://reviews.llvm.org/D63161



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

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

[Kristóf Umann via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1609-1613
+  if (B->rbegin()->getKind() != CFGElement::Kind::Statement)
+return nullptr;
+
+  // This should be the condition of the terminator block.
+  const Stmt *S = B->rbegin()->castAs().getStmt();

NoQ wrote:
> A bit clearner:
> 
> ```lang=c++
> auto StmtElem = B->rbegin().getAs();
> if (!StmtElem)
>   return nullptr;
> 
> const Stmt *S = StmtElem->getStmt();
> ```
> 
> Also how about `CFGBlock::getTerminatorCondition()`?
I peeked at it's implementation, and it seems to be incorrect.

Referencing @xazax.hun's inline:
> I vaguely recall some problem with the results of getTerminatorStmt for 
> logical operators like &&. I believe what we really want is the last 
> expression we evaluated in the basic block which will be the last Stmt of the 
> basic block. So if we can settle with the last stmt we can get rid of this 
> code.

And this is what `CFGBlock::getTerminatorCondition()` does:
```lang=c++
Stmt *Terminator = getTerminatorStmt();
if (!Terminator)
  return nullptr;
 
Expr *E = nullptr;

switch (Terminator->getStmtClass()) {
  // ...
  case Stmt::BinaryOperatorClass: // '&&' and '||'
E = cast(Terminator)->getLHS();
  // 
}
return E;
```
But nevertheless, maybe it'd be good to fix this in there and use it here.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1640-1642
+  CFGBlock *OriginB = GetRelevantBlock(Origin);
+  if (!OriginB || !NB)
+return nullptr;

NoQ wrote:
> `// TODO: This can be cached.`
Like, caching the `CFGBlock`s for given `ExplodedNode`s?



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1646
+if (const Expr *Condition = getTerminatorCondition(NB))
+  if (BR.addTrackedCondition(Condition))
+bugreporter::trackExpressionValue(

NoQ wrote:
> All right, i still don't understand this caching based on condition 
> expression.
> 
> You mean, like, if we're encountering the same condition multiple times (say, 
> in a loop), we should only track it once? Why? Like, its values (which are 
> the thing we'll really be tracking) may be different (say, on different loop 
> iterations).
Yup, can't argue with that, I'll revise this.


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

https://reviews.llvm.org/D62883



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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

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



Comment at: lib/Sema/SemaExpr.cpp:10931
+  // Do not diagnose hexadecimal literals
+  if (ExprStr.find("0x") != llvm::StringRef::npos)
+return;

Quuxplusone wrote:
> Can you use `starts_with` (or the LLVM equivalent) in both of these cases? 
> It'll be faster and also more correct.
> 
> Hex and binary are handled up here on line 10927, but octal is handled down 
> on line 10955; why? Can't they be combined into one place in the code?
We cannot use starts_with here, case: 2 ^ 0b11.

Yes, I can combine it to one place.




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

https://reviews.llvm.org/D63423



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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

xbolva00 added a comment.

Thanks, I will address your notes soon!

The only remaining question is, as your said, whether or not to diagnose xors 
in macros. @regerh @jfb ?


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

https://reviews.llvm.org/D63423



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

[PATCH] D60974: Clang IFSO driver action.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 205191.
plotfi marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/InterfaceStubs/bad-format.cpp
  clang/test/InterfaceStubs/class-template-specialization.cpp
  clang/test/InterfaceStubs/externstatic.c
  clang/test/InterfaceStubs/function-template-specialization.cpp
  clang/test/InterfaceStubs/hidden-class-inheritance.cpp
  clang/test/InterfaceStubs/inline.c
  clang/test/InterfaceStubs/inline.h
  clang/test/InterfaceStubs/object.cpp
  clang/test/InterfaceStubs/template-namespace-function.cpp
  clang/test/InterfaceStubs/virtual.cpp
  clang/test/InterfaceStubs/visibility.cpp
  clang/test/InterfaceStubs/weak.cpp

Index: clang/test/InterfaceStubs/weak.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/weak.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck %s
+
+// RUN: %clang -target x86_64-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-YAML %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-nm - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// CHECK: Symbols:
+// CHECK-DAG:  _Z8weakFuncv: { Type: Func, Weak: true }
+// CHECK-DAG:  _Z10strongFuncv: { Type: Func }
+
+// CHECK-YAML: Symbols:
+// CHECK-YAML-DAG:   - Name:_Z8weakFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_WEAK
+// CHECK-YAML-DAG:   - Name:_Z10strongFuncv
+// CHECK-YAML-DAG: Type:STT_FUNC
+// CHECK-YAML-DAG: Binding: STB_GLOBAL
+
+// CHECK-SYMBOLS-DAG: _Z10strongFuncv
+// CHECK-SYMBOLS-DAG: _Z8weakFuncv
+__attribute__((weak)) void weakFunc() {}
+int strongFunc() { return 42; }
Index: clang/test/InterfaceStubs/visibility.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/visibility.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 -fvisibility=hidden \
+// RUN: %s | FileCheck --check-prefix=CHECK-CMD-HIDDEN %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-tapi-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD2 %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: -interface-stub-version=experimental-yaml-elf-v1 %s | \
+// RUN: FileCheck --check-prefix=CHECK-CMD2 %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -c %s | llvm-readelf -s - 2>&1 | \
+// RUN: FileCheck -check-prefix=CHECK-SYMBOLS %s
+
+// Always Be Hidden:
+// CHECK-CMD-HIDDEN-NOT: _Z6hiddenv
+// CHECK-CMD2-NOT: _Z6hiddenv
+__attribute__((visibility("hidden"))) void hidden() {}
+
+// Always Be Visible:
+// CHECK-CMD-HIDDEN: _Z9nothiddenv
+// CHECK-CMD-DAG: _Z9nothiddenv
+__attribute__((visibility("default"))) void nothidden() {}
+
+// Do Whatever -fvisibility says:
+// CHECK-CMD-HIDDEN-NOT: _Z10cmdVisiblev
+// CHECK-CMD-DAG: _Z10cmdVisiblev
+void cmdVisible() {}
+
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z10cmdVisiblev
+// CHECK-SYMBOLS-DAG: HIDDEN {{.*}} _Z6hiddenv
+// CHECK-SYMBOLS-DAG: DEFAULT{{.*}} _Z9nothiddenv
Index: clang/test/InterfaceStubs/virtual.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/virtual.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang -target x86_64-unknown-linux-gnu -o - -emit-interface-stubs \
+// RUN: 

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:10931
+  // Do not diagnose hexadecimal literals
+  if (ExprStr.find("0x") != llvm::StringRef::npos)
+return;

Can you use `starts_with` (or the LLVM equivalent) in both of these cases? 
It'll be faster and also more correct.

Hex and binary are handled up here on line 10927, but octal is handled down on 
line 10955; why? Can't they be combined into one place in the code?



Comment at: lib/Sema/SemaExpr.cpp:10982
+S.Diag(Loc, diag::note_xor_used_as_pow_silence) << ("0x2 ^ " + RHSStr);
+  } else if (LeftSideValue == 10 && RightSideIntValue != 0) {
+std::string SuggestedValue;

Suppressing the warning specifically on `10 ^ 0` (which means `10`) seems like 
an anti-feature.



Comment at: lib/Sema/SemaExpr.cpp:10988
+} else {
+  SuggestedValue = "1" + std::string(RightSideIntValue, '0');
+}

I suggest at least one unit test case that involves `10 ^ 100`, and considering 
the user-friendliness of the resulting error message. How about suggesting 
`"1e" + std::to_string(RightSideIntValue)` as the fixit in both cases?



Comment at: test/SemaCXX/warn-xor-as-pow.cpp:52
+  res = FOOBAR(2, 16);
+  res = EPSILON;
+  res = 0b10 ^ 16;

I still expect to see a warning either on this line, or on the line where the 
macro is defined. I don't see why we'd want to be silent in this case.


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

https://reviews.llvm.org/D63423



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

r363622 - Fix crash when checking a dependently-typed reference that is

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Jun 17 14:46:17 2019
New Revision: 363622

URL: http://llvm.org/viewvc/llvm-project?rev=363622=rev
Log:
Fix crash when checking a dependently-typed reference that is
initialized from a non-value-dependent initializer.

Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/test/SemaTemplate/dependent-expr.cpp

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=363622=363621=363622=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Mon Jun 17 14:46:17 2019
@@ -2282,7 +2282,7 @@ bool VarDecl::isUsableInConstantExpressi
   //   declaration is encountered...
   const VarDecl *DefVD = nullptr;
   const Expr *Init = getAnyInitializer(DefVD);
-  if (!Init || Init->isValueDependent())
+  if (!Init || Init->isValueDependent() || getType()->isDependentType())
 return false;
   //   ... if it is a constexpr variable, or it is of reference type or of
   //   const-qualified integral or enumeration type, ...

Modified: cfe/trunk/test/SemaTemplate/dependent-expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/dependent-expr.cpp?rev=363622=363621=363622=diff
==
--- cfe/trunk/test/SemaTemplate/dependent-expr.cpp (original)
+++ cfe/trunk/test/SemaTemplate/dependent-expr.cpp Mon Jun 17 14:46:17 2019
@@ -63,6 +63,14 @@ namespace test5 {
   };
 }
 
+namespace test6 {
+  template T f() {
+const T (0);
+return v;
+  }
+  int use = f();
+}
+
 namespace PR8795 {
   template  int test(_CharT t)
   {


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

[PATCH] D63031: DebugInfo: Render the canonical name of a class template specialization, even when nested in another class template specialization

[David Blaikie via Phabricator via cfe-commits]

dblaikie marked an inline comment as done.
dblaikie added a comment.

Ping (just curious about the change to the canonicalization printing policy - 
making sure I don't break something important)


Repository:
  rC Clang

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

https://reviews.llvm.org/D63031



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

[PATCH] D62883: [analyzer] Track conditions of terminator statements on which the reported node depends on

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1609-1613
+  if (B->rbegin()->getKind() != CFGElement::Kind::Statement)
+return nullptr;
+
+  // This should be the condition of the terminator block.
+  const Stmt *S = B->rbegin()->castAs().getStmt();

A bit clearner:

```lang=c++
auto StmtElem = B->rbegin().getAs();
if (!StmtElem)
  return nullptr;

const Stmt *S = StmtElem->getStmt();
```

Also how about `CFGBlock::getTerminatorCondition()`?



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1640-1642
+  CFGBlock *OriginB = GetRelevantBlock(Origin);
+  if (!OriginB || !NB)
+return nullptr;

`// TODO: This can be cached.`



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1646
+if (const Expr *Condition = getTerminatorCondition(NB))
+  if (BR.addTrackedCondition(Condition))
+bugreporter::trackExpressionValue(

All right, i still don't understand this caching based on condition expression.

You mean, like, if we're encountering the same condition multiple times (say, 
in a loop), we should only track it once? Why? Like, its values (which are the 
thing we'll really be tracking) may be different (say, on different loop 
iterations).


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

https://reviews.llvm.org/D62883



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

[PATCH] D63454: [OpenMP] Strengthen regression tests for task allocation under nowait depend clauses NFC

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 205186.
gtbercea added a comment.

- Fix test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63454

Files:
  test/OpenMP/target_constant_device_codegen.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_enter_data_depend_codegen.cpp
  test/OpenMP/target_exit_data_depend_codegen.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_update_depend_codegen.cpp

Index: test/OpenMP/target_update_depend_codegen.cpp
===
--- test/OpenMP/target_update_depend_codegen.cpp
+++ test/OpenMP/target_update_depend_codegen.cpp
@@ -63,7 +63,9 @@
   // CK1: [[CAP_DEVICE:%.+]] = getelementptr inbounds %struct.anon, %struct.anon* [[CAPTURES:%.+]], i32 0, i32 0
   // CK1: [[DEVICE:%.+]] = load i32, i32* %{{.+}}
   // CK1: store i32 [[DEVICE]], i32* [[CAP_DEVICE]],
-  // CK1: [[RES:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* {{.+}}, i32 {{.+}}, i32 1, i[[sz]] [[sz]], i[[sz]] 4, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* [[TASK_ENTRY0:@.+]] to i32 (i32, i8*)*), i64
+  // CK1: [[DEV1:%.+]] = load i32, i32* %{{.+}}
+  // CK1: [[DEV2:%.+]] = sext i32 [[DEV1]] to i64
+  // CK1: [[RES:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* {{.+}}, i32 {{.+}}, i32 1, i[[sz]] [[sz]], i[[sz]] 4, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* [[TASK_ENTRY0:@.+]] to i32 (i32, i8*)*), i64 [[DEV2]])
   // CK1: [[BC:%.+]] = bitcast i8* [[RES]] to %struct.kmp_task_t_with_privates*
   // CK1: [[TASK_T:%.+]] = getelementptr inbounds %struct.kmp_task_t_with_privates, %struct.kmp_task_t_with_privates* [[BC]], i32 0, i32 0
   // CK1: [[SHAREDS:%.+]] = getelementptr inbounds %struct.kmp_task_t, %struct.kmp_task_t* [[TASK_T]], i32 0, i32 0
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -132,8 +132,10 @@
   // CHECK:   [[GEP:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 2
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
+  // CHECK:   [[DEV1:%.+]] = load i32, i32* [[DEVICE_CAP]],
+  // CHECK:   [[DEV2:%.+]] = sext i32 [[DEV1]] to i64
 
-  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{104|52}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1_:@.+]] to i32 (i32, i8*)*), i64
+  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{104|52}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1_:@.+]] to i32 (i32, i8*)*), i64 [[DEV2]])
   // CHECK:   [[BC_TASK:%.+]] = bitcast i8* [[TASK]] to [[TASK_TY1_:%.+]]*
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 0
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 1
@@ -148,8 +150,10 @@
   // CHECK:   [[GEP:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 2
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
+  // CHECK:   [[DEV1:%.+]] = load i32, i32* [[DEVICE_CAP]],
+  // CHECK:   [[DEV2:%.+]] = sext i32 [[DEV1]] to i64
 
-  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{56|28}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1__:@.+]] to i32 (i32, i8*)*), i64
+  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{56|28}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1__:@.+]] to i32 (i32, i8*)*), i64 [[DEV2]])
   // CHECK:   [[BC_TASK:%.+]] = bitcast i8* [[TASK]] to [[TASK_TY1__:%.+]]*
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 0
   // CHECK:   getelementptr inbounds [3 x 

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

xbolva00 updated this revision to Diff 205187.
xbolva00 added a comment.

Handle 10 ^ -C case..


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,90 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define FOOBAR(x, y) (x * y)
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+#define EPSILON 10^-300
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ -1;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+  res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0, maybe you mean '1<<2' (4)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<2"
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1<<16"
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
+  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<= width of type}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL<<32"
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '0x2 ^ 64' to silence this warning}}
+
+  res = 10 ^ 0;
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"10"
+  // expected-note@-2 {{replace expression with '0xA ^ 1' to silence this warning}}
+  res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 2' to silence this warning}}
+  res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14, maybe you mean '1'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1"
+  // expected-note@-2 {{replace expression with '0xA ^ 4' to silence this warning}}
+  res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = 0xA ^ 10;
+  #if defined(KW_XOR)
+  res = 10 xor 10;
+  #endif
+  res = XOR(10, 10);
+  res = 10 ^ -7; // expected-warning {{result of '10 ^ -7' is 13, maybe you mean '1e-7'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1e-7"
+  // expected-note@-2 {{replace expression with '0xA ^ -7' to silence this warning}}
+  res = 10 ^ (-7);
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ 

r363620 - Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Mon Jun 17 14:08:30 2019
New Revision: 363620

URL: http://llvm.org/viewvc/llvm-project?rev=363620=rev
Log:
Rewrite ConstStructBuilder with a mechanism that can cope with splitting and 
updating constants.

Summary:
This adds a ConstantBuilder class that deals with incrementally building
an aggregate constant, including support for overwriting
previously-emitted parts of the aggregate with new values.

This fixes a bunch of cases where we used to be unable to reduce a
DesignatedInitUpdateExpr down to an IR constant, and also lays some
groundwork for emission of class constants with [[no_unique_address]]
members.

Reviewers: rjmccall

Subscribers: cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/CodeGenCXX/designated-init.cpp
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=363620=363619=363620=diff
==
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Mon Jun 17 14:08:30 2019
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,345 +32,636 @@ using namespace clang;
 using namespace CodeGen;
 
 
//===--===//
-//ConstStructBuilder
+//ConstantAggregateBuilder
 
//===--===//
 
 namespace {
 class ConstExprEmitter;
-class ConstStructBuilder {
-  CodeGenModule 
-  ConstantEmitter 
-
-  bool Packed;
-  CharUnits NextFieldOffsetInChars;
-  CharUnits LLVMStructAlignment;
-  SmallVector Elements;
-public:
-  static llvm::Constant *BuildStruct(ConstantEmitter ,
- ConstExprEmitter *ExprEmitter,
- llvm::Constant *Base,
- InitListExpr *Updater,
- QualType ValTy);
-  static llvm::Constant *BuildStruct(ConstantEmitter ,
- InitListExpr *ILE, QualType StructTy);
-  static llvm::Constant *BuildStruct(ConstantEmitter ,
- const APValue , QualType ValTy);
-
-private:
-  ConstStructBuilder(ConstantEmitter )
-: CGM(emitter.CGM), Emitter(emitter), Packed(false),
-NextFieldOffsetInChars(CharUnits::Zero()),
-LLVMStructAlignment(CharUnits::One()) { }
-
-  void AppendField(const FieldDecl *Field, uint64_t FieldOffset,
-   llvm::Constant *InitExpr);
-
-  void AppendBytes(CharUnits FieldOffsetInChars, llvm::Constant *InitCst);
-
-  void AppendBitField(const FieldDecl *Field, uint64_t FieldOffset,
-  llvm::ConstantInt *InitExpr);
 
-  void AppendPadding(CharUnits PadSize);
-
-  void AppendTailPadding(CharUnits RecordSize);
-
-  void ConvertStructToPacked();
+struct ConstantAggregateBuilderUtils {
+  CodeGenModule 
 
-  bool Build(InitListExpr *ILE);
-  bool Build(ConstExprEmitter *Emitter, llvm::Constant *Base,
- InitListExpr *Updater);
-  bool Build(const APValue , const RecordDecl *RD, bool IsPrimaryBase,
- const CXXRecordDecl *VTableClass, CharUnits BaseOffset);
-  llvm::Constant *Finalize(QualType Ty);
+  ConstantAggregateBuilderUtils(CodeGenModule ) : CGM(CGM) {}
 
   CharUnits getAlignment(const llvm::Constant *C) const {
-if (Packed)  return CharUnits::One();
 return CharUnits::fromQuantity(
 CGM.getDataLayout().getABITypeAlignment(C->getType()));
   }
 
-  CharUnits getSizeInChars(const llvm::Constant *C) const {
-return CharUnits::fromQuantity(
-CGM.getDataLayout().getTypeAllocSize(C->getType()));
+  CharUnits getSize(llvm::Type *Ty) const {
+return CharUnits::fromQuantity(CGM.getDataLayout().getTypeAllocSize(Ty));
   }
-};
-
-void ConstStructBuilder::
-AppendField(const FieldDecl *Field, uint64_t FieldOffset,
-llvm::Constant *InitCst) {
-  const ASTContext  = CGM.getContext();
-
-  CharUnits FieldOffsetInChars = Context.toCharUnitsFromBits(FieldOffset);
-
-  AppendBytes(FieldOffsetInChars, InitCst);
-}
-
-void ConstStructBuilder::
-AppendBytes(CharUnits FieldOffsetInChars, llvm::Constant *InitCst) {
 
-  assert(NextFieldOffsetInChars <= FieldOffsetInChars
- && "Field offset mismatch!");
-
-  CharUnits FieldAlignment = getAlignment(InitCst);
-
-  // Round up the field offset to the alignment of the field type.
-  CharUnits AlignedNextFieldOffsetInChars =
-  NextFieldOffsetInChars.alignTo(FieldAlignment);
-
-  if 

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

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

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL363620: Rewrite ConstStructBuilder with a mechanism that can 
cope with splitting and… (authored by rsmith, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63371?vs=205182=205184#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63371

Files:
  cfe/trunk/lib/CodeGen/CGExprConstant.cpp
  cfe/trunk/test/CodeGenCXX/designated-init.cpp

Index: cfe/trunk/test/CodeGenCXX/designated-init.cpp
===
--- cfe/trunk/test/CodeGenCXX/designated-init.cpp
+++ cfe/trunk/test/CodeGenCXX/designated-init.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -std=c++98 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+
+struct A { int x, y[3]; };
+struct B { A a; };
+
+// CHECK: @b = global %{{[^ ]*}} { %{{[^ ]*}} { i32 1, [3 x i32] [i32 2, i32 5, i32 4] } }
+B b = {(A){1, 2, 3, 4}, .a.y[1] = 5};
+
+union U {
+  int n;
+  float f;
+};
+struct C {
+  int x;
+  U u[3];
+};
+struct D {
+  C c;
+};
+
+// CHECK: @d1 = {{.*}} { i32 1, [3 x %[[U:.*]]] [%[[U]] { i32 2 }, %[[U]] { i32 5 }, %[[U]] { i32 4 }] }
+D d1 = {(C){1, {{.n=2}, {.f=3}, {.n=4}}}, .c.u[1].n = 5};
+
+// CHECK: @d2 = {{.*}} { i32 1, { %[[U]], float, %[[U]] } { %[[U]] { i32 2 }, float 5.{{0*}}e+00, %[[U]] { i32 4 } } }
+D d2 = {(C){1, 2, 3, 4}, .c.u[1].f = 5};
+
+struct Bitfield {
+  int a : 3;
+  int b : 4;
+  int c : 5;
+};
+struct WithBitfield {
+  int n;
+  Bitfield b;
+};
+// CHECK: @bitfield = {{.*}} { i32 1, { i8, i8, [2 x i8] } { i8 42, i8 2, [2 x i8] undef } }
+WithBitfield bitfield = {1, (Bitfield){2, 3, 4}, .b.b = 5};
+
+struct String {
+  const char buffer[12];
+};
+struct WithString {
+  String str;
+};
+// CHECK: @string = {{.*}} [12 x i8] c"Hello World\00" } }
+WithString string = {(String){"hello world"}, .str.buffer[0] = 'H', .str.buffer[6] = 'W'};
+
+struct LargeArray {
+  int arr[4096];
+};
+struct WithLargeArray {
+  LargeArray arr;
+};
+// CHECK: @large = global { { <{ [11 x i32], [4085 x i32] }> } } { { <{ [11 x i32], [4085 x i32] }> } { <{ [11 x i32], [4085 x i32] }> <{ [11 x i32] [i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 10], [4085 x i32] zeroinitializer }> } }
+WithLargeArray large = {(LargeArray){1, 2, 3}, .arr.arr[10] = 10};
+
+union OverwritePaddingWithBitfield {
+  struct Padding { unsigned : 8; char c; } padding;
+  char bitfield : 3;
+};
+struct WithOverwritePaddingWithBitfield {
+  OverwritePaddingWithBitfield a;
+};
+// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } }
+WithOverwritePaddingWithBitfield overwrite_padding = {(OverwritePaddingWithBitfield){1}, .a.bitfield = 3};
Index: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,345 +32,636 @@
 using namespace CodeGen;
 
 //===--===//
-//ConstStructBuilder
+//ConstantAggregateBuilder
 //===--===//
 
 namespace {
 class ConstExprEmitter;
-class ConstStructBuilder {
-  CodeGenModule 
-  ConstantEmitter 
-
-  bool Packed;
-  CharUnits NextFieldOffsetInChars;
-  CharUnits LLVMStructAlignment;
-  SmallVector Elements;
-public:
-  static llvm::Constant *BuildStruct(ConstantEmitter ,
- ConstExprEmitter *ExprEmitter,
- llvm::Constant *Base,
- InitListExpr *Updater,
- QualType ValTy);
-  static llvm::Constant *BuildStruct(ConstantEmitter ,
- InitListExpr *ILE, QualType StructTy);
-  static llvm::Constant *BuildStruct(ConstantEmitter ,
- const APValue , QualType ValTy);
-
-private:
-  ConstStructBuilder(ConstantEmitter )
-: CGM(emitter.CGM), Emitter(emitter), Packed(false),
-NextFieldOffsetInChars(CharUnits::Zero()),
-LLVMStructAlignment(CharUnits::One()) { }
-
-  void AppendField(const FieldDecl *Field, uint64_t FieldOffset,
-   llvm::Constant *InitExpr);
-
-  void AppendBytes(CharUnits 

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

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

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



Comment at: lib/CodeGen/CGExprConstant.cpp:162
+  return replace(V, BeginOff, EndOff, Vals.begin(), Vals.end());
+}
+

rjmccall wrote:
> Can these go to STLExtras or somewhere similar?
Done. The use of offsets here is a bit special-case, so I've moved an iterator 
version to STLExtras and just left an offsets -> iterators adaptor here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:287
 .replace('\\}', '}') \
+.replace('', '\\') \
 .replace('\\<', '<') \

Charusso wrote:
> Ugh, wait with that a little-bit. I have forgot to create a patch! We have to 
> escape the escapes.
> E.g. `"\x42"` -> `"\\x42"`.
Yay!


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

https://reviews.llvm.org/D62761



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

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

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

rsmith updated this revision to Diff 205182.
rsmith marked 2 inline comments as done.
rsmith added a comment.

- Address additional review comments from rjmccall.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/designated-init.cpp

Index: test/CodeGenCXX/designated-init.cpp
===
--- /dev/null
+++ test/CodeGenCXX/designated-init.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -std=c++98 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+
+struct A { int x, y[3]; };
+struct B { A a; };
+
+// CHECK: @b = global %{{[^ ]*}} { %{{[^ ]*}} { i32 1, [3 x i32] [i32 2, i32 5, i32 4] } }
+B b = {(A){1, 2, 3, 4}, .a.y[1] = 5};
+
+union U {
+  int n;
+  float f;
+};
+struct C {
+  int x;
+  U u[3];
+};
+struct D {
+  C c;
+};
+
+// CHECK: @d1 = {{.*}} { i32 1, [3 x %[[U:.*]]] [%[[U]] { i32 2 }, %[[U]] { i32 5 }, %[[U]] { i32 4 }] }
+D d1 = {(C){1, {{.n=2}, {.f=3}, {.n=4}}}, .c.u[1].n = 5};
+
+// CHECK: @d2 = {{.*}} { i32 1, { %[[U]], float, %[[U]] } { %[[U]] { i32 2 }, float 5.{{0*}}e+00, %[[U]] { i32 4 } } }
+D d2 = {(C){1, 2, 3, 4}, .c.u[1].f = 5};
+
+struct Bitfield {
+  int a : 3;
+  int b : 4;
+  int c : 5;
+};
+struct WithBitfield {
+  int n;
+  Bitfield b;
+};
+// CHECK: @bitfield = {{.*}} { i32 1, { i8, i8, [2 x i8] } { i8 42, i8 2, [2 x i8] undef } }
+WithBitfield bitfield = {1, (Bitfield){2, 3, 4}, .b.b = 5};
+
+struct String {
+  const char buffer[12];
+};
+struct WithString {
+  String str;
+};
+// CHECK: @string = {{.*}} [12 x i8] c"Hello World\00" } }
+WithString string = {(String){"hello world"}, .str.buffer[0] = 'H', .str.buffer[6] = 'W'};
+
+struct LargeArray {
+  int arr[4096];
+};
+struct WithLargeArray {
+  LargeArray arr;
+};
+// CHECK: @large = global { { <{ [11 x i32], [4085 x i32] }> } } { { <{ [11 x i32], [4085 x i32] }> } { <{ [11 x i32], [4085 x i32] }> <{ [11 x i32] [i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 10], [4085 x i32] zeroinitializer }> } }
+WithLargeArray large = {(LargeArray){1, 2, 3}, .arr.arr[10] = 10};
+
+union OverwritePaddingWithBitfield {
+  struct Padding { unsigned : 8; char c; } padding;
+  char bitfield : 3;
+};
+struct WithOverwritePaddingWithBitfield {
+  OverwritePaddingWithBitfield a;
+};
+// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } }
+WithOverwritePaddingWithBitfield overwrite_padding = {(OverwritePaddingWithBitfield){1}, .a.bitfield = 3};
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,148 +32,562 @@
 using namespace CodeGen;
 
 //===--===//
-//ConstStructBuilder
+//ConstantAggregateBuilder
 //===--===//
 
 namespace {
 class ConstExprEmitter;
+
+struct ConstantAggregateBuilderUtils {
+  CodeGenModule 
+
+  ConstantAggregateBuilderUtils(CodeGenModule ) : CGM(CGM) {}
+
+  CharUnits getAlignment(const llvm::Constant *C) const {
+return CharUnits::fromQuantity(
+CGM.getDataLayout().getABITypeAlignment(C->getType()));
+  }
+
+  CharUnits getSize(llvm::Type *Ty) const {
+return CharUnits::fromQuantity(CGM.getDataLayout().getTypeAllocSize(Ty));
+  }
+
+  CharUnits getSize(const llvm::Constant *C) const {
+return getSize(C->getType());
+  }
+
+  llvm::Constant *getPadding(CharUnits PadSize) const {
+llvm::Type *Ty = CGM.Int8Ty;
+if (PadSize > CharUnits::One())
+  Ty = llvm::ArrayType::get(Ty, PadSize.getQuantity());
+return llvm::UndefValue::get(Ty);
+  }
+
+  llvm::Constant *getZeroes(CharUnits ZeroSize) const {
+llvm::Type *Ty = llvm::ArrayType::get(CGM.Int8Ty, ZeroSize.getQuantity());
+return llvm::ConstantAggregateZero::get(Ty);
+  }
+};
+
+/// Incremental builder for an llvm::Constant* holding a struct or array
+/// constant.
+class ConstantAggregateBuilder : private ConstantAggregateBuilderUtils {
+  /// The elements of the constant. These two arrays must have the same size;
+  /// Offsets[i] describes the offset of Elems[i] within the constant. The
+  /// elements are kept in increasing offset order, and we ensure that there
+  /// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]).
+  ///
+  /// This may contain explicit padding elements (in order to 

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/utils/analyzer/exploded-graph-rewriter.py:287
 .replace('\\}', '}') \
+.replace('', '\\') \
 .replace('\\<', '<') \

Ugh, wait with that a little-bit. I have forgot to create a patch! We have to 
escape the escapes.
E.g. `"\x42"` -> `"\\x42"`.


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

https://reviews.llvm.org/D62761



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

[PATCH] D62761: [analyzer] exploded-graph-rewriter: Implement a --diff mode.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 205177.
NoQ added a comment.

Add tests, rebase.


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

https://reviews.llvm.org/D62761

Files:
  clang/test/Analysis/exploded-graph-rewriter/environment_diff.dot
  clang/test/Analysis/exploded-graph-rewriter/program_points.dot
  clang/test/Analysis/exploded-graph-rewriter/store.dot
  clang/test/Analysis/exploded-graph-rewriter/store_diff.dot
  clang/utils/analyzer/exploded-graph-rewriter.py

Index: clang/utils/analyzer/exploded-graph-rewriter.py
===
--- clang/utils/analyzer/exploded-graph-rewriter.py
+++ clang/utils/analyzer/exploded-graph-rewriter.py
@@ -18,6 +18,13 @@
 import re
 
 
+# A helper function for finding the difference between two dictionaries.
+def diff_dicts(curr, prev):
+removed = [k for k in prev if k not in curr or curr[k] != prev[k]]
+added = [k for k in curr if k not in prev or curr[k] != prev[k]]
+return (removed, added)
+
+
 # A deserialized source location.
 class SourceLocation(object):
 def __init__(self, json_loc):
@@ -47,13 +54,21 @@
 self.block_id = json_pp['block_id']
 
 
-# A value of a single expression in a deserialized Environment.
-class EnvironmentBinding(object):
-def __init__(self, json_eb):
-super(EnvironmentBinding, self).__init__()
-self.stmt_id = json_eb['stmt_id']
-self.pretty = json_eb['pretty']
-self.value = json_eb['value']
+# A single expression acting as a key in a deserialized Environment.
+class EnvironmentBindingKey(object):
+def __init__(self, json_ek):
+super(EnvironmentBindingKey, self).__init__()
+self.stmt_id = json_ek['stmt_id']
+self.pretty = json_ek['pretty']
+
+def _key(self):
+return self.stmt_id
+
+def __eq__(self, other):
+return self._key() == other._key()
+
+def __hash__(self):
+return hash(self._key())
 
 
 # Deserialized description of a location context.
@@ -65,6 +80,15 @@
 self.decl = json_frame['calling']
 self.line = json_frame['call_line']
 
+def _key(self):
+return self.lctx_id
+
+def __eq__(self, other):
+return self._key() == other._key()
+
+def __hash__(self):
+return hash(self._key())
+
 
 # A group of deserialized Environment bindings that correspond to a specific
 # location context.
@@ -72,8 +96,17 @@
 def __init__(self, json_frame):
 super(EnvironmentFrame, self).__init__()
 self.location_context = LocationContext(json_frame)
-self.bindings = [EnvironmentBinding(b) for b in json_frame['items']] \
-if json_frame['items'] is not None else []
+self.bindings = collections.OrderedDict(
+[(EnvironmentBindingKey(b),
+  b['value']) for b in json_frame['items']]
+if json_frame['items'] is not None else [])
+
+def diff_bindings(self, prev):
+return diff_dicts(self.bindings, prev.bindings)
+
+def is_different(self, prev):
+removed, added = self.diff_bindings(prev)
+return len(removed) != 0 or len(added) != 0
 
 
 # A deserialized Environment.
@@ -82,14 +115,46 @@
 super(Environment, self).__init__()
 self.frames = [EnvironmentFrame(f) for f in json_e]
 
+def diff_frames(self, prev):
+# TODO: It's difficult to display a good diff when frame numbers shift.
+if len(self.frames) != len(prev.frames):
+return None
+
+updated = []
+for i in range(len(self.frames)):
+f = self.frames[i]
+prev_f = prev.frames[i]
+if f.location_context == prev_f.location_context:
+if f.is_different(prev_f):
+updated.append(i)
+else:
+# We have the whole frame replaced with another frame.
+# TODO: Produce a nice diff.
+return None
+
+# TODO: Add support for added/removed.
+return updated
+
+def is_different(self, prev):
+updated = self.diff_frames(prev)
+return updated is None or len(updated) > 0
+
+
+# A single binding key in a deserialized RegionStore cluster.
+class StoreBindingKey(object):
+def __init__(self, json_sk):
+super(StoreBindingKey, self).__init__()
+self.kind = json_sk['kind']
+self.offset = json_sk['offset']
 
-# A single binding in a deserialized RegionStore cluster.
-class StoreBinding(object):
-def __init__(self, json_sb):
-super(StoreBinding, self).__init__()
-self.kind = json_sb['kind']
-self.offset = json_sb['offset']
-self.value = json_sb['value']
+def _key(self):
+return (self.kind, self.offset)
+
+def __eq__(self, other):
+return self._key() == other._key()
+
+def __hash__(self):
+return hash(self._key())
 
 
 # A single cluster of the deserialized RegionStore.
@@ 

[PATCH] D63290: Unify DependencyFileGenerator class and DependencyCollector interface

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a reviewer: benlangmuir.
aganea added inline comments.



Comment at: clang/include/clang/Frontend/Utils.h:118
 
 /// Builds a depdenency file when attached to a Preprocessor (for includes) and
 /// ASTReader (for module imports), and writes it out at the end of processing

s/depdenency/dependency/



Comment at: clang/lib/Frontend/DependencyFile.cpp:78
   SrcMgr::CharacteristicKind FileType) override {
 if (!File)
   DepCollector.maybeAddDependency(FileName, /*FromModule*/false,

Given that `llvm::sys::path::remove_leading_dotslash` is not called here, but 
it is for the function above, could you end up with two entries in 
`DependencyCollector::Seen` when `DependencyCollector::sawDependency` is 
overriden?



Comment at: clang/lib/Frontend/DependencyFile.cpp:165
bool IsMissing) {
-  return !isSpecialFilename(Filename) &&
+  return !IsMissing && !isSpecialFilename(Filename) &&
  (needSystemDependencies() || !IsSystem);

Why `!IsMissing` wasn't considered before? Just wondering.


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

https://reviews.llvm.org/D63290



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

[PATCH] D63367: [clang-doc] Add basic support for templates and typedef

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 205169.
DiegoAstiazaran marked an inline comment as done.
DiegoAstiazaran added a comment.

Specify type in declarations where //auto// keyword was used.
Initialize pointer to nullptr in empty declaration.


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

https://reviews.llvm.org/D63367

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.h
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp
  clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -142,7 +142,15 @@
   E() {}
 protected:
   void ProtectedMethod();
-};)raw", 3, /*Public=*/false, Infos);
+};
+template 
+struct F {
+  void TemplateMethod();
+};
+template <>
+void F::TemplateMethod();
+typedef struct {} G;)raw",
+   7, /*Public=*/false, Infos);
 
   RecordInfo *E = InfoAsRecord(Infos[0].get());
   RecordInfo ExpectedE(EmptySID, "E");
@@ -176,6 +184,51 @@
   Method.IsMethod = true;
   ExpectedRecordWithMethod.ChildFunctions.emplace_back(std::move(Method));
   CheckRecordInfo(, RecordWithMethod);
+
+  RecordInfo *F = InfoAsRecord(Infos[3].get());
+  RecordInfo ExpectedF(EmptySID, "F");
+  ExpectedF.TagType = TagTypeKind::TTK_Struct;
+  ExpectedF.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  CheckRecordInfo(, F);
+
+  RecordInfo *RecordWithTemplateMethod = InfoAsRecord(Infos[4].get());
+  RecordInfo ExpectedRecordWithTemplateMethod(EmptySID);
+  FunctionInfo TemplateMethod;
+  TemplateMethod.Name = "TemplateMethod";
+  TemplateMethod.Parent = Reference(EmptySID, "F", InfoType::IT_record);
+  TemplateMethod.ReturnType = TypeInfo(EmptySID, "void", InfoType::IT_default);
+  TemplateMethod.Loc.emplace_back(0, llvm::SmallString<16>{"test.cpp"});
+  TemplateMethod.Namespace.emplace_back(EmptySID, "F", InfoType::IT_record);
+  TemplateMethod.Access = AccessSpecifier::AS_public;
+  TemplateMethod.IsMethod = true;
+  ExpectedRecordWithTemplateMethod.ChildFunctions.emplace_back(
+  std::move(TemplateMethod));
+  CheckRecordInfo(, RecordWithTemplateMethod);
+
+  RecordInfo *TemplatedRecord = InfoAsRecord(Infos[5].get());
+  RecordInfo ExpectedTemplatedRecord(EmptySID);
+  FunctionInfo SpecializedTemplateMethod;
+  SpecializedTemplateMethod.Name = "TemplateMethod";
+  SpecializedTemplateMethod.Parent =
+  Reference(EmptySID, "F", InfoType::IT_record);
+  SpecializedTemplateMethod.ReturnType =
+  TypeInfo(EmptySID, "void", InfoType::IT_default);
+  SpecializedTemplateMethod.Loc.emplace_back(0,
+ llvm::SmallString<16>{"test.cpp"});
+  SpecializedTemplateMethod.Namespace.emplace_back(EmptySID, "F",
+   InfoType::IT_record);
+  SpecializedTemplateMethod.Access = AccessSpecifier::AS_public;
+  SpecializedTemplateMethod.IsMethod = true;
+  ExpectedTemplatedRecord.ChildFunctions.emplace_back(
+  std::move(SpecializedTemplateMethod));
+  CheckRecordInfo(, TemplatedRecord);
+
+  RecordInfo *G = InfoAsRecord(Infos[6].get());
+  RecordInfo ExpectedG(EmptySID, "G");
+  ExpectedG.TagType = TagTypeKind::TTK_Struct;
+  ExpectedG.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  ExpectedG.IsTypeDef = true;
+  CheckRecordInfo(, G);
 }
 
 // Test serialization of enum declarations.
@@ -284,9 +337,13 @@
 
 TEST(SerializeTest, emitInheritedRecordInfo) {
   EmittedInfoList Infos;
-  ExtractInfosFromCode(
-  "class F {}; class G{} ; class E : public F, virtual private G {};", 3,
-  /*Public=*/false, Infos);
+  ExtractInfosFromCode(R"raw(class F {};
+class G {} ;
+class E : public F, virtual private G {};
+template 
+class H {} ;
+class I : public H {} ;)raw",
+   5, /*Public=*/false, Infos);
 
   RecordInfo *F = InfoAsRecord(Infos[0].get());
   RecordInfo ExpectedF(EmptySID, "F");
@@ -307,6 +364,19 @@
   ExpectedE.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
   ExpectedE.TagType = TagTypeKind::TTK_Class;
   CheckRecordInfo(, E);
+
+  RecordInfo *H = InfoAsRecord(Infos[3].get());
+  RecordInfo ExpectedH(EmptySID, "H");
+  ExpectedH.TagType = TagTypeKind::TTK_Class;
+  ExpectedH.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  CheckRecordInfo(, H);
+
+  RecordInfo *I = InfoAsRecord(Infos[4].get());
+  RecordInfo ExpectedI(EmptySID, "I");
+  ExpectedI.Parents.emplace_back(EmptySID, "H", InfoType::IT_record);
+  ExpectedI.DefLoc = Location(0, llvm::SmallString<16>{"test.cpp"});
+  ExpectedI.TagType = TagTypeKind::TTK_Class;
+  

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Minor requests, then LGTM.




Comment at: lib/CodeGen/CGExprConstant.cpp:73
+/// Incremental builder for an llvm::Constant* holding a structure constant.
+class ConstantBuilder : private ConstantBuilderUtils {
+  llvm::SmallVector Elems;

rsmith wrote:
> rjmccall wrote:
> > This seems like a very generic name for this type.
> It is intended to be a very generic type. (I was trying to arrange it so that 
> it could possibly be moved to LLVM eventually. I heard that globalopt would 
> benefit from being able to do this kind of constant splitting / reforming.) 
> Is `ConstantAggregateBuilder` sufficiently more precise?
Yeah, that makes sense, since it aligns with the LLVM type name.  I guess the 
big blocker there is the lack of a `CharUnits` equivalent in LLVM.



Comment at: lib/CodeGen/CGExprConstant.cpp:190
+bool ConstantBuilder::addBits(llvm::APInt Bits, uint64_t OffsetInBits,
+  bool AllowOverwrite) {
+  const ASTContext  = CGM.getContext();

rsmith wrote:
> rjmccall wrote:
> > `AllowOversized` (which you used in the interface) seems like a better name.
> `AllowOversized` is used to mean "the size of the constant may be larger than 
> the size of the type", and is a parameter to `build` / `buildFrom`.
> `AllowOverwrite` is used to mean "adding this constant may overwrite 
> something you've already been given", and is a parameter to `add` / `addBits`.
> 
> I can make these names more different from each other if that would help?
Oh, oops.  No, sorry, no need to do anything here.



Comment at: lib/CodeGen/CGExprConstant.cpp:258
+  return false;
+assert(CI->getBitWidth() == CharWidth && "splitAt failed");
+assert((!(CI->getValue() & UpdateMask) || AllowOverwrite) &&

rsmith wrote:
> rjmccall wrote:
> > Oh, because we're splitting before and after a single-`CharUnits` range?  
> > That seems worthy of a somewhat clearer explanation in the code.
> > 
> > I guess we could have a non-`ConstantInt` single-byte value.  Unlikely but 
> > not impossible. :)
> Yes. It's not too hard to craft a testcase where we'd get an explicit `undef` 
> here; added handling for that and for all-zeroes constants, which are both 
> correctly handled by overwriting the whole byte.
Ah, yeah, good catch; I was thinking it was okay to be suboptimal if we saw  a 
`<1 x i8>` or a 1-byte pointer or whatever, but undef and zero are definitely 
worth covering here.



Comment at: lib/CodeGen/CGExprConstant.cpp:375
+
+  // FIXME: We could theoretically split a ConstantInt if the need ever arose.
+

rsmith wrote:
> rjmccall wrote:
> > Does this not come up all the time with bit-fields?  I guess we emit them 
> > in single-`char` chunks, so it wouldn't.  Probably worth a comment.
> Done.
> 
> We could hit this case for cases such as:
> 
> ```
> union U { int a; int b : 3; };
> struct S { U u; };
> S s = {(union U){1234}, .u.b = 5};
> ```
> 
> (which `CodeGen` currently rejects with "cannot compile this static 
> initializer yet" in C), and splitting the `ConstantInt` would allow us to 
> emit that initializer as a constant, but I'm not sure it's worthwhile unless 
> it lets us simplify or improve bitfield emission in general. (The above isn't 
> a case that C requires us to treat as a constant initializer, so rejecting it 
> is not a conformance issue.)
> 
> Maybe instead of splitting bitfields into 1-byte chunks like we currently do, 
> we should try to combine them into a single `iN`, like 
> `CGRecordLayoutBuilder` does. But splitting to `i8` maintains the status quo, 
> which is what I was aiming for in this patch.
I'm fine with single-byte emission for constant bit-fields; ugliness here 
shouldn't really have significant consequences.  The comment is good enough.



Comment at: lib/CodeGen/CGExprConstant.cpp:162
+  return replace(V, BeginOff, EndOff, Vals.begin(), Vals.end());
+}
+

Can these go to STLExtras or somewhere similar?



Comment at: lib/CodeGen/CGExprConstant.cpp:221
+// bits have unspecified values.
+llvm::APInt BitsThisByte = Bits;
+if (BitsThisByte.getBitWidth() < CharWidth)

Should this be `BitsThisChar` for consistency?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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

[PATCH] D63367: [clang-doc] Add basic support for templates and typedef

[Diego Astiazarán via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-doc/Representation.h:244
  // interface).
+  bool IsTypeDef = false; // Indicates if record was declared using typedef
   llvm::SmallVector

juliehockett wrote:
> Run clang-format? Not totally sure what it'll do, but I think it should line 
> up the comments.
There are no changes after running clang-format.


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

https://reviews.llvm.org/D63367



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

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

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

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



Comment at: lib/CodeGen/CGExprConstant.cpp:967
 // Constant folding is currently missing support for a few features supported
 // here: CK_ToUnion, CK_ReinterpretMemberPointer, and DesignatedInitUpdateExpr.
 class ConstExprEmitter :

efriedma wrote:
> Like the comment here says, we're awfully close to being able to just 
> completely kill off ConstExprEmitter etc. Do we really want to continue to 
> invest effort into this code?
The new code is also the mechanism by which we emit `APValue` constants, which 
is the replacement for `ConstExprEmitter`.

Also I don't think we have a fix for the performance issues we saw from 
performing frontend evaluation of large `InitListExpr`s yet, which is another 
blocker for removing `ConstExprEmitter` in favor of `APValue` emission (though 
I don't expect that to be a problem forever; `APValue` is long overdue an 
overhaul).


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:13345-13346
+<< (FD->getStorageClass() == SC_None
+? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(),
+ "static ")
+: FixItHint{});

aaron.ballman wrote:
> aaronpuchert wrote:
> > aaron.ballman wrote:
> > > We may not want to produce the fixit if there's a macro involved in the 
> > > declaration. Consider:
> > > ```
> > > #ifdef SOMETHING
> > > #define FROBBLE static
> > > #else
> > > #define FROBBLE
> > > #endif
> > > 
> > > FROBBLE void foo(void);
> > > ```
> > > We probably don't want the fixit in the case `SOMETHING` is not defined.
> > I think that's generally an issue with fix-its, there could always be a 
> > macro that turns the code into something entirely different. If we look at 
> > the other fix-it above, we can construct
> > 
> > ```
> > #define VOID
> > int f(VOID);
> > int f() { return 0; }
> > ```
> > 
> > Then we get:
> > ```
> > :3:5: warning: no previous prototype for function 'f' 
> > [-Wmissing-prototypes]
> > int f() { return 0; }
> > ^
> > :2:5: note: this declaration is not a prototype; add 'void' to make 
> > it a prototype for a zero-parameter function
> > int f(VOID);
> > ^
> >   void
> > ```
> > 
> > Then the fix-it doesn't even work in the original configuration, because it 
> > produces `int f(VOIDvoid)`. If we make it work by adding a space, we still 
> > have the problem that you mentioned: if someone defines `VOID` as `void`, 
> > we then have `int f(void void)` after applying the fix-it in the original 
> > setting.
> > 
> > Trying to make fix-its work with macros is probably a hopeless endeavor.
> > Trying to make fix-its work with macros is probably a hopeless endeavor.
> 
> That's what I was getting at -- I think you need to see if there's a macro at 
> the insertion location and not generate a fix-it in that case. The above 
> suffers from the same issue (and can be corrected in a subsequent patch, if 
> desired).
How can I know that? The AST seems to contain no such information, for the 
example I only see the following:

```
TranslationUnitDecl 0x12574e8 <> 
|-[...]
|-FunctionDecl 0x12b5340 <:2:1, col:11> col:5 f 'int ()'
`-FunctionDecl 0x12b5440 prev 0x12b5340  col:5 f 'int ()'
  `-CompoundStmt 0x12b5508 
`-ReturnStmt 0x12b54f8 
  `-IntegerLiteral 0x12b54d8  'int' 0
```

It seems fix-its are automatically suppressed if they would land in a macro 
definition, like

```
#define X int f()
X;
int f() { return 0; }
```

produces

```
:3:5: warning: no previous prototype for function 'f' 
[-Wmissing-prototypes]
int f() { return 0; }
^
:2:1: note: this declaration is not a prototype; add 'void' to make it a 
prototype for a zero-parameter function
X;
^
:1:15: note: expanded from macro 'X'
#define X int f()
  ^
```

So there is no fix-it suggested, although we produce one. I guess some 
intermediate layer that knows about the connection to the original source 
detects that and throws it away.

But what you want is that we also suppress it if there is any macro (it 
wouldn't need to be at the beginning, it could also be `void FROBBLE foo(void)` 
with the same meaning). That would also mean I can't apply the fix-it even if 
the macros is unrelated, say if someone has

```
#define ATTR_PURE __attribute__((pure))
ATTR_PURE int f() { return 0; }
```

Other fix-its (that I have looked at) just seem to ignore macros, like we do, 
relying on fix-its to be discarded if they would land inside of a macro.

I think there is a case to be made for avoiding `int f(VOIDvoid)`, but not 
suggesting the fix-it because a macro could have another meaning in another 
configuration seems unusual to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

[PATCH] D62944: [Driver] Fix wchar_t and wint_t definitions on Solaris

[Rainer Orth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363612: Clang :: Sema/wchar.c has long been failing on 
Solaris: (authored by ro, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62944?vs=205012=205163#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62944

Files:
  cfe/trunk/lib/Basic/Targets/OSTargets.h
  cfe/trunk/test/Preprocessor/wchar_t.c
  cfe/trunk/test/Sema/format-strings.c
  cfe/trunk/test/Sema/wchar.c


Index: cfe/trunk/test/Sema/format-strings.c
===
--- cfe/trunk/test/Sema/format-strings.c
+++ cfe/trunk/test/Sema/format-strings.c
@@ -329,7 +329,11 @@
   printf("%S", s); // no-warning
   printf("%s", s); // expected-warning{{format specifies type 'char *' but the 
argument has type 'wchar_t *'}}
   printf("%C", s[0]); // no-warning
+#if defined(__sun) && !defined(__LP64__)
+  printf("%c", s[0]); // expected-warning{{format specifies type 'int' but the 
argument has type 'wchar_t' (aka 'long')}}
+#else
   printf("%c", s[0]);
+#endif
   // FIXME: This test reports inconsistent results. On Windows, '%C' expects
   // 'unsigned short'.
   // printf("%C", 10);
@@ -401,7 +405,7 @@
 void pr7981(wint_t c, wchar_t c2) {
   printf("%lc", c); // no-warning
   printf("%lc", 1.0); // expected-warning{{the argument has type 'double'}}
-#if __WINT_WIDTH__ == 32
+#if __WINT_WIDTH__ == 32 && !(defined(__sun) && !defined(__LP64__))
   printf("%lc", (char) 1); // no-warning
 #else
   printf("%lc", (char) 1); // expected-warning{{the argument has type 'char'}}
Index: cfe/trunk/test/Sema/wchar.c
===
--- cfe/trunk/test/Sema/wchar.c
+++ cfe/trunk/test/Sema/wchar.c
@@ -9,7 +9,11 @@
 #elif defined(__arm) || defined(__aarch64__)
   #define WCHAR_T_TYPE unsigned int
 #elif defined(__sun)
-  #define WCHAR_T_TYPE long
+  #if defined(__LP64__)
+#define WCHAR_T_TYPE int
+  #else
+#define WCHAR_T_TYPE long
+  #endif
 #else /* Solaris. */
   #define WCHAR_T_TYPE int
 #endif
Index: cfe/trunk/test/Preprocessor/wchar_t.c
===
--- cfe/trunk/test/Preprocessor/wchar_t.c
+++ cfe/trunk/test/Preprocessor/wchar_t.c
@@ -1,8 +1,13 @@
 // RUN: %clang_cc1 -triple i386-pc-solaris -dM -E %s -o - | FileCheck %s 
-check-prefix CHECK-SOLARIS
 // CHECK-SOLARIS-DAG: #define __WCHAR_MAX__ 2147483647
-// CHECK-SOLARIS-DAG: #define __WCHAR_TYPE__ int
+// CHECK-SOLARIS-DAG: #define __WCHAR_TYPE__ long int
 // CHECK-SOLARIS-NOT: #define __WCHAR_UNSIGNED__ 0
 
+// RUN: %clang_cc1 -triple x86_64-pc-solaris -dM -E %s -o - | FileCheck %s 
-check-prefix CHECK-SOLARIS64
+// CHECK-SOLARIS64-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-SOLARIS64-DAG: #define __WCHAR_TYPE__ int
+// CHECK-SOLARIS64-NOT: #define __WCHAR_UNSIGNED__ 0
+
 // RUN: %clang_cc1 -triple avr-unknown-unknown -fwchar-type=int -fsigned-wchar 
-dM -E %s -o - | FileCheck %s -check-prefix CHECK-AVR
 // CHECK-AVR-DAG: #define __WCHAR_MAX__ 32767
 // CHECK-AVR-DAG: #define __WCHAR_TYPE__ int
Index: cfe/trunk/lib/Basic/Targets/OSTargets.h
===
--- cfe/trunk/lib/Basic/Targets/OSTargets.h
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h
@@ -632,7 +632,11 @@
 public:
   SolarisTargetInfo(const llvm::Triple , const TargetOptions )
   : OSTargetInfo(Triple, Opts) {
-// FIXME: WIntType should be SignedLong
+if (this->PointerWidth == 64) {
+  this->WCharType = this->WIntType = this->SignedInt;
+} else {
+  this->WCharType = this->WIntType = this->SignedLong;
+}
 switch (Triple.getArch()) {
 default:
   break;


Index: cfe/trunk/test/Sema/format-strings.c
===
--- cfe/trunk/test/Sema/format-strings.c
+++ cfe/trunk/test/Sema/format-strings.c
@@ -329,7 +329,11 @@
   printf("%S", s); // no-warning
   printf("%s", s); // expected-warning{{format specifies type 'char *' but the argument has type 'wchar_t *'}}
   printf("%C", s[0]); // no-warning
+#if defined(__sun) && !defined(__LP64__)
+  printf("%c", s[0]); // expected-warning{{format specifies type 'int' but the argument has type 'wchar_t' (aka 'long')}}
+#else
   printf("%c", s[0]);
+#endif
   // FIXME: This test reports inconsistent results. On Windows, '%C' expects
   // 'unsigned short'.
   // printf("%C", 10);
@@ -401,7 +405,7 @@
 void pr7981(wint_t c, wchar_t c2) {
   printf("%lc", c); // no-warning
   printf("%lc", 1.0); // expected-warning{{the argument has type 'double'}}
-#if __WINT_WIDTH__ == 32
+#if __WINT_WIDTH__ == 32 && !(defined(__sun) && !defined(__LP64__))
   printf("%lc", (char) 1); // no-warning
 #else
   printf("%lc", (char) 1); // expected-warning{{the argument has type 

r363612 - Clang :: Sema/wchar.c has long been failing on Solaris:

[Rainer Orth via cfe-commits]

Author: ro
Date: Mon Jun 17 13:21:25 2019
New Revision: 363612

URL: http://llvm.org/viewvc/llvm-project?rev=363612=rev
Log:
Clang :: Sema/wchar.c has long been failing on Solaris:

  error: 'error' diagnostics expected but not seen: 
File /vol/llvm/src/clang/local/test/Sema/wchar.c Line 22: initializing wide 
char array with non-wide string literal
  error: 'error' diagnostics seen but not expected: 
File /vol/llvm/src/clang/local/test/Sema/wchar.c Line 20: array initializer 
must be an initializer list
File /vol/llvm/src/clang/local/test/Sema/wchar.c Line 22: array initializer 
must be an initializer list

It turns out the definition is wrong, as can be seen in GCC's gcc/config/sol2.h:

  /* wchar_t is called differently in  for 32 and 64-bit
 compilations.  This is called for by SCD 2.4.1, p. 6-83, Figure 6-65
 (32-bit) and p. 6P-10, Figure 6.38 (64-bit).  */
  
  #undef WCHAR_TYPE
  #define WCHAR_TYPE (TARGET_64BIT ? "int" : "long int")

The following patch implements this, and at the same time corrects the wint_t
definition which is the same:

  /* Same for wint_t.  See SCD 2.4.1, p. 6-83, Figure 6-66 (32-bit).  There's
 no corresponding 64-bit definition, but this is what Solaris 8
  uses.  */
  
  #undef WINT_TYPE
  #define WINT_TYPE (TARGET_64BIT ? "int" : "long int")

Clang :: Preprocessor/wchar_t.c and Clang :: Sema/format-strings.c need to
be adjusted to account for that.

Tested on i386-pc-solaris2.11, x86_64-pc-solaris2.11, and x86_64-pc-linux-gnu.

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

Modified:
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/test/Preprocessor/wchar_t.c
cfe/trunk/test/Sema/format-strings.c
cfe/trunk/test/Sema/wchar.c

Modified: cfe/trunk/lib/Basic/Targets/OSTargets.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/OSTargets.h?rev=363612=363611=363612=diff
==
--- cfe/trunk/lib/Basic/Targets/OSTargets.h (original)
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h Mon Jun 17 13:21:25 2019
@@ -632,7 +632,11 @@ protected:
 public:
   SolarisTargetInfo(const llvm::Triple , const TargetOptions )
   : OSTargetInfo(Triple, Opts) {
-// FIXME: WIntType should be SignedLong
+if (this->PointerWidth == 64) {
+  this->WCharType = this->WIntType = this->SignedInt;
+} else {
+  this->WCharType = this->WIntType = this->SignedLong;
+}
 switch (Triple.getArch()) {
 default:
   break;

Modified: cfe/trunk/test/Preprocessor/wchar_t.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/wchar_t.c?rev=363612=363611=363612=diff
==
--- cfe/trunk/test/Preprocessor/wchar_t.c (original)
+++ cfe/trunk/test/Preprocessor/wchar_t.c Mon Jun 17 13:21:25 2019
@@ -1,8 +1,13 @@
 // RUN: %clang_cc1 -triple i386-pc-solaris -dM -E %s -o - | FileCheck %s 
-check-prefix CHECK-SOLARIS
 // CHECK-SOLARIS-DAG: #define __WCHAR_MAX__ 2147483647
-// CHECK-SOLARIS-DAG: #define __WCHAR_TYPE__ int
+// CHECK-SOLARIS-DAG: #define __WCHAR_TYPE__ long int
 // CHECK-SOLARIS-NOT: #define __WCHAR_UNSIGNED__ 0
 
+// RUN: %clang_cc1 -triple x86_64-pc-solaris -dM -E %s -o - | FileCheck %s 
-check-prefix CHECK-SOLARIS64
+// CHECK-SOLARIS64-DAG: #define __WCHAR_MAX__ 2147483647
+// CHECK-SOLARIS64-DAG: #define __WCHAR_TYPE__ int
+// CHECK-SOLARIS64-NOT: #define __WCHAR_UNSIGNED__ 0
+
 // RUN: %clang_cc1 -triple avr-unknown-unknown -fwchar-type=int -fsigned-wchar 
-dM -E %s -o - | FileCheck %s -check-prefix CHECK-AVR
 // CHECK-AVR-DAG: #define __WCHAR_MAX__ 32767
 // CHECK-AVR-DAG: #define __WCHAR_TYPE__ int

Modified: cfe/trunk/test/Sema/format-strings.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/format-strings.c?rev=363612=363611=363612=diff
==
--- cfe/trunk/test/Sema/format-strings.c (original)
+++ cfe/trunk/test/Sema/format-strings.c Mon Jun 17 13:21:25 2019
@@ -329,7 +329,11 @@ void test_unicode_conversions(wchar_t *s
   printf("%S", s); // no-warning
   printf("%s", s); // expected-warning{{format specifies type 'char *' but the 
argument has type 'wchar_t *'}}
   printf("%C", s[0]); // no-warning
+#if defined(__sun) && !defined(__LP64__)
+  printf("%c", s[0]); // expected-warning{{format specifies type 'int' but the 
argument has type 'wchar_t' (aka 'long')}}
+#else
   printf("%c", s[0]);
+#endif
   // FIXME: This test reports inconsistent results. On Windows, '%C' expects
   // 'unsigned short'.
   // printf("%C", 10);
@@ -401,7 +405,7 @@ void bug7377_bad_length_mod_usage() {
 void pr7981(wint_t c, wchar_t c2) {
   printf("%lc", c); // no-warning
   printf("%lc", 1.0); // expected-warning{{the argument has type 'double'}}
-#if __WINT_WIDTH__ == 32
+#if __WINT_WIDTH__ == 32 && !(defined(__sun) && !defined(__LP64__))
   printf("%lc", (char) 1); // 

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

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

rsmith updated this revision to Diff 205162.
rsmith added a comment.

- Fix somewhat-incorrect comment on Size member.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/designated-init.cpp

Index: test/CodeGenCXX/designated-init.cpp
===
--- /dev/null
+++ test/CodeGenCXX/designated-init.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -std=c++98 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+
+struct A { int x, y[3]; };
+struct B { A a; };
+
+// CHECK: @b = global %{{[^ ]*}} { %{{[^ ]*}} { i32 1, [3 x i32] [i32 2, i32 5, i32 4] } }
+B b = {(A){1, 2, 3, 4}, .a.y[1] = 5};
+
+union U {
+  int n;
+  float f;
+};
+struct C {
+  int x;
+  U u[3];
+};
+struct D {
+  C c;
+};
+
+// CHECK: @d1 = {{.*}} { i32 1, [3 x %[[U:.*]]] [%[[U]] { i32 2 }, %[[U]] { i32 5 }, %[[U]] { i32 4 }] }
+D d1 = {(C){1, {{.n=2}, {.f=3}, {.n=4}}}, .c.u[1].n = 5};
+
+// CHECK: @d2 = {{.*}} { i32 1, { %[[U]], float, %[[U]] } { %[[U]] { i32 2 }, float 5.{{0*}}e+00, %[[U]] { i32 4 } } }
+D d2 = {(C){1, 2, 3, 4}, .c.u[1].f = 5};
+
+struct Bitfield {
+  int a : 3;
+  int b : 4;
+  int c : 5;
+};
+struct WithBitfield {
+  int n;
+  Bitfield b;
+};
+// CHECK: @bitfield = {{.*}} { i32 1, { i8, i8, [2 x i8] } { i8 42, i8 2, [2 x i8] undef } }
+WithBitfield bitfield = {1, (Bitfield){2, 3, 4}, .b.b = 5};
+
+struct String {
+  const char buffer[12];
+};
+struct WithString {
+  String str;
+};
+// CHECK: @string = {{.*}} [12 x i8] c"Hello World\00" } }
+WithString string = {(String){"hello world"}, .str.buffer[0] = 'H', .str.buffer[6] = 'W'};
+
+struct LargeArray {
+  int arr[4096];
+};
+struct WithLargeArray {
+  LargeArray arr;
+};
+// CHECK: @large = global { { <{ [11 x i32], [4085 x i32] }> } } { { <{ [11 x i32], [4085 x i32] }> } { <{ [11 x i32], [4085 x i32] }> <{ [11 x i32] [i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 10], [4085 x i32] zeroinitializer }> } }
+WithLargeArray large = {(LargeArray){1, 2, 3}, .arr.arr[10] = 10};
+
+union OverwritePaddingWithBitfield {
+  struct Padding { unsigned : 8; char c; } padding;
+  char bitfield : 3;
+};
+struct WithOverwritePaddingWithBitfield {
+  OverwritePaddingWithBitfield a;
+};
+// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } }
+WithOverwritePaddingWithBitfield overwrite_padding = {(OverwritePaddingWithBitfield){1}, .a.bitfield = 3};
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,148 +32,585 @@
 using namespace CodeGen;
 
 //===--===//
-//ConstStructBuilder
+//ConstantAggregateBuilder
 //===--===//
 
 namespace {
 class ConstExprEmitter;
+
+struct ConstantAggregateBuilderUtils {
+  CodeGenModule 
+
+  ConstantAggregateBuilderUtils(CodeGenModule ) : CGM(CGM) {}
+
+  CharUnits getAlignment(const llvm::Constant *C) const {
+return CharUnits::fromQuantity(
+CGM.getDataLayout().getABITypeAlignment(C->getType()));
+  }
+
+  CharUnits getSize(llvm::Type *Ty) const {
+return CharUnits::fromQuantity(CGM.getDataLayout().getTypeAllocSize(Ty));
+  }
+
+  CharUnits getSize(const llvm::Constant *C) const {
+return getSize(C->getType());
+  }
+
+  llvm::Constant *getPadding(CharUnits PadSize) const {
+llvm::Type *Ty = CGM.Int8Ty;
+if (PadSize > CharUnits::One())
+  Ty = llvm::ArrayType::get(Ty, PadSize.getQuantity());
+return llvm::UndefValue::get(Ty);
+  }
+
+  llvm::Constant *getZeroes(CharUnits ZeroSize) const {
+llvm::Type *Ty = llvm::ArrayType::get(CGM.Int8Ty, ZeroSize.getQuantity());
+return llvm::ConstantAggregateZero::get(Ty);
+  }
+};
+
+/// Incremental builder for an llvm::Constant* holding a struct or array
+/// constant.
+class ConstantAggregateBuilder : private ConstantAggregateBuilderUtils {
+  /// The elements of the constant. These two arrays must have the same size;
+  /// Offsets[i] describes the offset of Elems[i] within the constant. The
+  /// elements are kept in increasing offset order, and we ensure that there
+  /// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]).
+  ///
+  /// This may contain explicit padding elements (in order to create a
+  /// natural layout), but need not. 

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:967
 // Constant folding is currently missing support for a few features supported
 // here: CK_ToUnion, CK_ReinterpretMemberPointer, and DesignatedInitUpdateExpr.
 class ConstExprEmitter :

Like the comment here says, we're awfully close to being able to just 
completely kill off ConstExprEmitter etc. Do we really want to continue to 
invest effort into this code?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

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

rsmith added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:73
+/// Incremental builder for an llvm::Constant* holding a structure constant.
+class ConstantBuilder : private ConstantBuilderUtils {
+  llvm::SmallVector Elems;

rjmccall wrote:
> This seems like a very generic name for this type.
It is intended to be a very generic type. (I was trying to arrange it so that 
it could possibly be moved to LLVM eventually. I heard that globalopt would 
benefit from being able to do this kind of constant splitting / reforming.) Is 
`ConstantAggregateBuilder` sufficiently more precise?



Comment at: lib/CodeGen/CGExprConstant.cpp:75
+  llvm::SmallVector Elems;
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();

rjmccall wrote:
> Are there invariants about these?  I assume they're parallel arrays; are they 
> kept sorted?
Added comments to explain.



Comment at: lib/CodeGen/CGExprConstant.cpp:76
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();
+

rjmccall wrote:
> This is one past the last byte that's been covered by an actual `Constant*` 
> value, or does it include unoccupied padding, or does it exclude even 
> occupied padding?
Added comment to explain.



Comment at: lib/CodeGen/CGExprConstant.cpp:98
+Offsets.reserve(N);
+  }
+

rjmccall wrote:
> Might be worth clarifying what `N` is here.
Looks like this ended up being unused; removed.



Comment at: lib/CodeGen/CGExprConstant.cpp:190
+bool ConstantBuilder::addBits(llvm::APInt Bits, uint64_t OffsetInBits,
+  bool AllowOverwrite) {
+  const ASTContext  = CGM.getContext();

rjmccall wrote:
> `AllowOversized` (which you used in the interface) seems like a better name.
`AllowOversized` is used to mean "the size of the constant may be larger than 
the size of the type", and is a parameter to `build` / `buildFrom`.
`AllowOverwrite` is used to mean "adding this constant may overwrite something 
you've already been given", and is a parameter to `add` / `addBits`.

I can make these names more different from each other if that would help?



Comment at: lib/CodeGen/CGExprConstant.cpp:258
+  return false;
+assert(CI->getBitWidth() == CharWidth && "splitAt failed");
+assert((!(CI->getValue() & UpdateMask) || AllowOverwrite) &&

rjmccall wrote:
> Oh, because we're splitting before and after a single-`CharUnits` range?  
> That seems worthy of a somewhat clearer explanation in the code.
> 
> I guess we could have a non-`ConstantInt` single-byte value.  Unlikely but 
> not impossible. :)
Yes. It's not too hard to craft a testcase where we'd get an explicit `undef` 
here; added handling for that and for all-zeroes constants, which are both 
correctly handled by overwriting the whole byte.



Comment at: lib/CodeGen/CGExprConstant.cpp:375
+
+  // FIXME: We could theoretically split a ConstantInt if the need ever arose.
+

rjmccall wrote:
> Does this not come up all the time with bit-fields?  I guess we emit them in 
> single-`char` chunks, so it wouldn't.  Probably worth a comment.
Done.

We could hit this case for cases such as:

```
union U { int a; int b : 3; };
struct S { U u; };
S s = {(union U){1234}, .u.b = 5};
```

(which `CodeGen` currently rejects with "cannot compile this static initializer 
yet" in C), and splitting the `ConstantInt` would allow us to emit that 
initializer as a constant, but I'm not sure it's worthwhile unless it lets us 
simplify or improve bitfield emission in general. (The above isn't a case that 
C requires us to treat as a constant initializer, so rejecting it is not a 
conformance issue.)

Maybe instead of splitting bitfields into 1-byte chunks like we currently do, 
we should try to combine them into a single `iN`, like `CGRecordLayoutBuilder` 
does. But splitting to `i8` maintains the status quo, which is what I was 
aiming for in this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

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

rsmith updated this revision to Diff 205161.
rsmith marked 13 inline comments as done.
rsmith added a comment.

- Address review comments from rjmccall.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371

Files:
  lib/CodeGen/CGExprConstant.cpp
  test/CodeGenCXX/designated-init.cpp

Index: test/CodeGenCXX/designated-init.cpp
===
--- /dev/null
+++ test/CodeGenCXX/designated-init.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -std=c++98 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm -o - %s -triple x86_64-linux-gnu | FileCheck %s
+
+struct A { int x, y[3]; };
+struct B { A a; };
+
+// CHECK: @b = global %{{[^ ]*}} { %{{[^ ]*}} { i32 1, [3 x i32] [i32 2, i32 5, i32 4] } }
+B b = {(A){1, 2, 3, 4}, .a.y[1] = 5};
+
+union U {
+  int n;
+  float f;
+};
+struct C {
+  int x;
+  U u[3];
+};
+struct D {
+  C c;
+};
+
+// CHECK: @d1 = {{.*}} { i32 1, [3 x %[[U:.*]]] [%[[U]] { i32 2 }, %[[U]] { i32 5 }, %[[U]] { i32 4 }] }
+D d1 = {(C){1, {{.n=2}, {.f=3}, {.n=4}}}, .c.u[1].n = 5};
+
+// CHECK: @d2 = {{.*}} { i32 1, { %[[U]], float, %[[U]] } { %[[U]] { i32 2 }, float 5.{{0*}}e+00, %[[U]] { i32 4 } } }
+D d2 = {(C){1, 2, 3, 4}, .c.u[1].f = 5};
+
+struct Bitfield {
+  int a : 3;
+  int b : 4;
+  int c : 5;
+};
+struct WithBitfield {
+  int n;
+  Bitfield b;
+};
+// CHECK: @bitfield = {{.*}} { i32 1, { i8, i8, [2 x i8] } { i8 42, i8 2, [2 x i8] undef } }
+WithBitfield bitfield = {1, (Bitfield){2, 3, 4}, .b.b = 5};
+
+struct String {
+  const char buffer[12];
+};
+struct WithString {
+  String str;
+};
+// CHECK: @string = {{.*}} [12 x i8] c"Hello World\00" } }
+WithString string = {(String){"hello world"}, .str.buffer[0] = 'H', .str.buffer[6] = 'W'};
+
+struct LargeArray {
+  int arr[4096];
+};
+struct WithLargeArray {
+  LargeArray arr;
+};
+// CHECK: @large = global { { <{ [11 x i32], [4085 x i32] }> } } { { <{ [11 x i32], [4085 x i32] }> } { <{ [11 x i32], [4085 x i32] }> <{ [11 x i32] [i32 1, i32 2, i32 3, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 0, i32 10], [4085 x i32] zeroinitializer }> } }
+WithLargeArray large = {(LargeArray){1, 2, 3}, .arr.arr[10] = 10};
+
+union OverwritePaddingWithBitfield {
+  struct Padding { unsigned : 8; char c; } padding;
+  char bitfield : 3;
+};
+struct WithOverwritePaddingWithBitfield {
+  OverwritePaddingWithBitfield a;
+};
+// CHECK: @overwrite_padding = global { { i8, i8 } } { { i8, i8 } { i8 3, i8 1 } }
+WithOverwritePaddingWithBitfield overwrite_padding = {(OverwritePaddingWithBitfield){1}, .a.bitfield = 3};
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -22,6 +22,8 @@
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Builtins.h"
+#include "llvm/ADT/Sequence.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
@@ -30,148 +32,583 @@
 using namespace CodeGen;
 
 //===--===//
-//ConstStructBuilder
+//ConstantAggregateBuilder
 //===--===//
 
 namespace {
 class ConstExprEmitter;
+
+struct ConstantAggregateBuilderUtils {
+  CodeGenModule 
+
+  ConstantAggregateBuilderUtils(CodeGenModule ) : CGM(CGM) {}
+
+  CharUnits getAlignment(const llvm::Constant *C) const {
+return CharUnits::fromQuantity(
+CGM.getDataLayout().getABITypeAlignment(C->getType()));
+  }
+
+  CharUnits getSize(llvm::Type *Ty) const {
+return CharUnits::fromQuantity(CGM.getDataLayout().getTypeAllocSize(Ty));
+  }
+
+  CharUnits getSize(const llvm::Constant *C) const {
+return getSize(C->getType());
+  }
+
+  llvm::Constant *getPadding(CharUnits PadSize) const {
+llvm::Type *Ty = CGM.Int8Ty;
+if (PadSize > CharUnits::One())
+  Ty = llvm::ArrayType::get(Ty, PadSize.getQuantity());
+return llvm::UndefValue::get(Ty);
+  }
+
+  llvm::Constant *getZeroes(CharUnits ZeroSize) const {
+llvm::Type *Ty = llvm::ArrayType::get(CGM.Int8Ty, ZeroSize.getQuantity());
+return llvm::ConstantAggregateZero::get(Ty);
+  }
+};
+
+/// Incremental builder for an llvm::Constant* holding a struct or array
+/// constant.
+class ConstantAggregateBuilder : private ConstantAggregateBuilderUtils {
+  /// The elements of the constant. These two arrays must have the same size;
+  /// Offsets[i] describes the offset of Elems[i] within the constant. The
+  /// elements are kept in increasing offset order, and we ensure that there
+  /// is no overlap: Offsets[i+1] >= Offsets[i] + getSize(Elemes[i]).
+  ///
+  /// This may contain explicit padding elements (in order to create a
+  /// 

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

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

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



Comment at: include/llvm/IR/IRBuilder.h:1138
+
+return MetadataAsValue::get(Context, RoundingMDS);
+  }

rjmccall wrote:
> kpn wrote:
> > rjmccall wrote:
> > > Huh?  You build an `MDNode` that wraps an `MDString` and then immediately 
> > > extract the `MDString` from it and drop the `MDNode`?
> > > 
> > > I think you should just have a function somewhere that returns the 
> > > correct metadata string for a `ConstrainedRoundingKind`, and then the 
> > > code here is much more obvious.  Maybe this can be wherever you declare 
> > > the enums.  You can also have a function that goes the other way and 
> > > returns an `Optional`.
> > > 
> > > Function name should include `FP`.
> > > 
> > > Same points apply to the next function.
> > Well, when you put it that way, your proposed change does result in much 
> > simpler code.
> > 
> > The resulting code isn't that much more than the switch. I hope it is now 
> > easy enough to read. These functions are private to the class and just 
> > exist to be helpers. I don't see a need to put the switch table into a 
> > different function, but I'm willing to be convinced. 
> I feel like having the enum->string mapping available somewhere independent 
> of `IRBuilder` will probably be useful to someone, and it nicely separates 
> concerns in any case .  You could just make it a static method on 
> `ConstrainedFPIntrinsic` along with the reverse operation.
Ok, will do.


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

https://reviews.llvm.org/D53157



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

[PATCH] D63454: [OpenMP] Strengthen regression tests for task allocation under nowait depend clauses NFC

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added inline comments.



Comment at: test/OpenMP/target_constant_device_codegen.cpp:2-4
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s 
--check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++11 -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -x c++ -triple powerpc64le-unknown-unknown 
-fopenmp-targets=powerpc64le-ibm-linux-gnu -std=c++11 -include-pch %t -verify 
%s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64

I think you only need these 3 lines (although not really sure about the PCH).



Comment at: test/OpenMP/target_constant_device_codegen.cpp:13-33
+template
+struct TT{
+  tx X;
+  ty Y;
+};
+
+int global;

Please only leave structs and variables that are really used and needed (in 
particular only one `depend`)



Comment at: test/OpenMP/target_constant_device_codegen.cpp:37
+
+  // CHECK: call i8* @__kmpc_omp_target_task_alloc({{.*}}, i64 -1)
+

Can you please also add another `target` region with a real clause + constant, 
ie `device(1)`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63454



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

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

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

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



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609
+def warn_mul_in_bool_context : Warning<
+  "'*' in bool context, maybe you mean '&&'?">,
+  InGroup;

aaron.ballman wrote:
> I would appreciate seeing some motivating examples for this case, because it 
> seems like the `&&` suggestion is misleading more often than it's helpful in 
> the current test cases.
> 
> Also, what is a "bool context"?
Bool context - places where we expect boolean expressions.

If you have idea for better terminology, I am happy to apply it.

Yeah, I will remove “&&” suggestion.. 



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5611
+  InGroup;
+def warn_left_shift_in_bool_context : Warning<
+  "'<<' in bool context, maybe you mean '<'?">,

Oh, inconsistency. I will use boolean context than bool context :)  (if no 
suggestions)



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5625
+  "evaluate to 'true'">,
+  InGroup;
+

aaron.ballman wrote:
> This one seems like it should be in the `TautologicalConstantCompare` group, 
> no?
Yes, we could move it there.


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

https://reviews.llvm.org/D63082



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

[PATCH] D60974: Clang IFSO driver action.

[Saleem Abdulrasool via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:91
+  std::vector MangledNames = CGNameGen.getAllManglings(ND);
+  if (isa(ND) || isa(ND)) {
+if (MangledNames.size() == 1)

I don't understand this clause.  The structurors will always have 2 manglings 
(both itanium and MSVC use multiple, I don't remember the SUN and GNU variants 
off the top of my head).  Adding the assert for the non-structor case is fine, 
but please wrap the entire thing in an `LLVM_EXPENSIVE_ASSERT`.



Comment at: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp:116
+if (RDO & IsLate) {
+  llvm_unreachable("Generating Interface Stubs is not supported with "
+   "delayed template parsing.");

This really should emit a diagnostic.  The `CompilerInstance` should have a 
reference to the `DiagnosticsEngine`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: include/llvm/IR/IRBuilder.h:1138
+
+return MetadataAsValue::get(Context, RoundingMDS);
+  }

kpn wrote:
> rjmccall wrote:
> > Huh?  You build an `MDNode` that wraps an `MDString` and then immediately 
> > extract the `MDString` from it and drop the `MDNode`?
> > 
> > I think you should just have a function somewhere that returns the correct 
> > metadata string for a `ConstrainedRoundingKind`, and then the code here is 
> > much more obvious.  Maybe this can be wherever you declare the enums.  You 
> > can also have a function that goes the other way and returns an 
> > `Optional`.
> > 
> > Function name should include `FP`.
> > 
> > Same points apply to the next function.
> Well, when you put it that way, your proposed change does result in much 
> simpler code.
> 
> The resulting code isn't that much more than the switch. I hope it is now 
> easy enough to read. These functions are private to the class and just exist 
> to be helpers. I don't see a need to put the switch table into a different 
> function, but I'm willing to be convinced. 
I feel like having the enum->string mapping available somewhere independent of 
`IRBuilder` will probably be useful to someone, and it nicely separates 
concerns in any case .  You could just make it a static method on 
`ConstrainedFPIntrinsic` along with the reverse operation.


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

https://reviews.llvm.org/D53157



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

[PATCH] D63454: [OpenMP] Strengthen regression tests for task allocation under nowait depend clauses NFC

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea created this revision.
gtbercea added reviewers: ABataev, Hahnfeld, caomhin.
Herald added subscribers: cfe-commits, jdoerfert, guansong.
Herald added a project: clang.

This patch strengthens the tests introduced in D63009 
 by:

- adding new test for default device ID.
- modifying existing tests to pass device ID local variable to the task 
allocation function.


Repository:
  rC Clang

https://reviews.llvm.org/D63454

Files:
  test/OpenMP/target_constant_device_codegen.cpp
  test/OpenMP/target_depend_codegen.cpp
  test/OpenMP/target_enter_data_depend_codegen.cpp
  test/OpenMP/target_exit_data_depend_codegen.cpp
  test/OpenMP/target_parallel_depend_codegen.cpp
  test/OpenMP/target_parallel_for_depend_codegen.cpp
  test/OpenMP/target_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_simd_depend_codegen.cpp
  test/OpenMP/target_teams_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_depend_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
  test/OpenMP/target_update_depend_codegen.cpp

Index: test/OpenMP/target_update_depend_codegen.cpp
===
--- test/OpenMP/target_update_depend_codegen.cpp
+++ test/OpenMP/target_update_depend_codegen.cpp
@@ -63,7 +63,9 @@
   // CK1: [[CAP_DEVICE:%.+]] = getelementptr inbounds %struct.anon, %struct.anon* [[CAPTURES:%.+]], i32 0, i32 0
   // CK1: [[DEVICE:%.+]] = load i32, i32* %{{.+}}
   // CK1: store i32 [[DEVICE]], i32* [[CAP_DEVICE]],
-  // CK1: [[RES:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* {{.+}}, i32 {{.+}}, i32 1, i[[sz]] [[sz]], i[[sz]] 4, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* [[TASK_ENTRY0:@.+]] to i32 (i32, i8*)*), i64
+  // CK1: [[DEV1:%.+]] = load i32, i32* %{{.+}}
+  // CK1: [[DEV2:%.+]] = sext i32 [[DEV1]] to i64
+  // CK1: [[RES:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* {{.+}}, i32 {{.+}}, i32 1, i[[sz]] [[sz]], i[[sz]] 4, i32 (i32, i8*)* bitcast (i32 (i32, %struct.kmp_task_t_with_privates*)* [[TASK_ENTRY0:@.+]] to i32 (i32, i8*)*), i64 [[DEV2]])
   // CK1: [[BC:%.+]] = bitcast i8* [[RES]] to %struct.kmp_task_t_with_privates*
   // CK1: [[TASK_T:%.+]] = getelementptr inbounds %struct.kmp_task_t_with_privates, %struct.kmp_task_t_with_privates* [[BC]], i32 0, i32 0
   // CK1: [[SHAREDS:%.+]] = getelementptr inbounds %struct.kmp_task_t, %struct.kmp_task_t* [[TASK_T]], i32 0, i32 0
Index: test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
+++ test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
@@ -132,8 +132,10 @@
   // CHECK:   [[GEP:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 2
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
+  // CHECK:   [[DEV1:%.+]] = load i32, i32* [[DEVICE_CAP]],
+  // CHECK:   [[DEV2:%.+]] = sext i32 [[DEV1]] to i64
 
-  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{104|52}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1_:@.+]] to i32 (i32, i8*)*), i64
+  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{104|52}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1_:@.+]] to i32 (i32, i8*)*), i64 [[DEV2]])
   // CHECK:   [[BC_TASK:%.+]] = bitcast i8* [[TASK]] to [[TASK_TY1_:%.+]]*
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 0
   // CHECK:   getelementptr inbounds [3 x %struct.kmp_depend_info], [3 x %struct.kmp_depend_info]* %{{.+}}, i[[SZ]] 0, i[[SZ]] 1
@@ -148,8 +150,10 @@
   // CHECK:   [[GEP:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* %{{.+}}, i32 0, i32 2
   // CHECK:   [[DEV:%.+]] = load i32, i32* [[DEVICE_CAP]],
   // CHECK:   store i32 [[DEV]], i32* [[GEP]],
+  // CHECK:   [[DEV1:%.+]] = load i32, i32* [[DEVICE_CAP]],
+  // CHECK:   [[DEV2:%.+]] = sext i32 [[DEV1]] to i64
 
-  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{56|28}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1__:@.+]] to i32 (i32, i8*)*), i64
+  // CHECK:   [[TASK:%.+]] = call i8* @__kmpc_omp_target_task_alloc(%struct.ident_t* [[ID]], i32 [[GTID]], i32 1, i[[SZ]] {{56|28}}, i[[SZ]] {{16|12}}, i32 (i32, i8*)* bitcast (i32 (i32, %{{.+}}*)* [[TASK_ENTRY1__:@.+]] to i32 (i32, i8*)*), i64 [[DEV2]])
   // CHECK:   

[PATCH] D62944: [Driver] Fix wchar_t and wint_t definitions on Solaris

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D62944



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

[PATCH] D63082: [Diagnostics] Added support for -Wint-in-bool-context

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5609
+def warn_mul_in_bool_context : Warning<
+  "'*' in bool context, maybe you mean '&&'?">,
+  InGroup;

I would appreciate seeing some motivating examples for this case, because it 
seems like the `&&` suggestion is misleading more often than it's helpful in 
the current test cases.

Also, what is a "bool context"?



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:5625
+  "evaluate to 'true'">,
+  InGroup;
+

This one seems like it should be in the `TautologicalConstantCompare` group, no?



Comment at: lib/Sema/SemaChecking.cpp:11099
 
+static void DiagnoseIntInBoolContext(Sema , Expr *E) {
+  E = E->IgnoreParenImpCasts();

`const Expr *E`?



Comment at: test/Sema/integer-overflow.c:37
 // expected-warning@+1 2{{overflow in expression; result is 536870912 with 
type 'int'}}
-  overflow = 4608 * 1024 * 1024 ?  4608 * 1024 * 1024 : 0;
+  overflow = 4608 * 1024 * 1024 ?  4608 * 1024 * 1024 : 0; // expected-warning 
{{'*' in bool context, maybe you mean '&&'?}}
 

I don't think the new diagnostic adds a lot of value here, though GCC does warn 
on it similarly. Truthfully, the same goes for the other test cases in this 
file.


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

https://reviews.llvm.org/D63082



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

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

kpn added inline comments.



Comment at: include/llvm/IR/IRBuilder.h:1138
+
+return MetadataAsValue::get(Context, RoundingMDS);
+  }

rjmccall wrote:
> Huh?  You build an `MDNode` that wraps an `MDString` and then immediately 
> extract the `MDString` from it and drop the `MDNode`?
> 
> I think you should just have a function somewhere that returns the correct 
> metadata string for a `ConstrainedRoundingKind`, and then the code here is 
> much more obvious.  Maybe this can be wherever you declare the enums.  You 
> can also have a function that goes the other way and returns an 
> `Optional`.
> 
> Function name should include `FP`.
> 
> Same points apply to the next function.
Well, when you put it that way, your proposed change does result in much 
simpler code.

The resulting code isn't that much more than the switch. I hope it is now easy 
enough to read. These functions are private to the class and just exist to be 
helpers. I don't see a need to put the switch table into a different function, 
but I'm willing to be convinced. 


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

https://reviews.llvm.org/D53157



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

[PATCH] D63436: [analyzer] Fix JSON dumps for ExplodedNodes

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/test/Analysis/dump_egraph.c:3
 // RUN: cat %t.dot | FileCheck %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-dump-egraph=%t.dot 
-trim-egraph %s
 // REQUIRES: asserts

Charusso wrote:
> This did nothing.
Let's fix it by adding another `// RUN: cat %t.dot | FileCheck %s` after this 
line instead, so that both trimmed graph and untrimmed graph gets tested.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63436



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

[PATCH] D53157: Teach the IRBuilder about constrained fadd and friends

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

kpn updated this revision to Diff 205151.
kpn marked 3 inline comments as done.
kpn added a comment.

Address review comments.

Run the changes through clang-format.


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

https://reviews.llvm.org/D53157

Files:
  include/llvm/IR/IRBuilder.h
  include/llvm/IR/IntrinsicInst.h
  unittests/IR/IRBuilderTest.cpp

Index: unittests/IR/IRBuilderTest.cpp
===
--- unittests/IR/IRBuilderTest.cpp
+++ unittests/IR/IRBuilderTest.cpp
@@ -122,6 +122,70 @@
   EXPECT_FALSE(II->hasNoNaNs());
 }
 
+TEST_F(IRBuilderTest, ConstrainedFP) {
+  IRBuilder<> Builder(BB);
+  Value *V;
+  CallInst *Call;
+  IntrinsicInst *II;
+
+  V = Builder.CreateLoad(GV);
+
+  // See if we get constrained intrinsics instead of non-constrained
+  // instructions.
+  Builder.setIsFPConstrained(true);
+
+  V = Builder.CreateFAdd(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+
+  V = Builder.CreateFSub(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fsub);
+
+  V = Builder.CreateFMul(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fmul);
+  
+  V = Builder.CreateFDiv(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_fdiv);
+  
+  V = Builder.CreateFRem(V, V);
+  ASSERT_TRUE(isa(V));
+  II = cast(V);
+  EXPECT_EQ(II->getIntrinsicID(), Intrinsic::experimental_constrained_frem);
+
+  // Verify the codepaths for setting and overriding the default metadata.
+  V = Builder.CreateFAdd(V, V);
+  ASSERT_TRUE(isa(V));
+  auto *CII = cast(V);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebStrict);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmDynamic);
+
+  Builder.setDefaultConstrainedExcept(ConstrainedFPIntrinsic::ebIgnore);
+  Builder.setDefaultConstrainedRounding(ConstrainedFPIntrinsic::rmUpward);
+  V = Builder.CreateFAdd(V, V);
+  CII = cast(V);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebIgnore);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmUpward);
+
+  // Now override the defaults.
+  Call = Builder.CreateConstrainedFPBinOp(
+Intrinsic::experimental_constrained_fadd, V, V, nullptr, "",
+ConstrainedFPIntrinsic::rmDownward, ConstrainedFPIntrinsic::ebMayTrap);
+  CII = cast(Call);
+  EXPECT_EQ(CII->getIntrinsicID(), Intrinsic::experimental_constrained_fadd);
+  ASSERT_TRUE(CII->getExceptionBehavior() == ConstrainedFPIntrinsic::ebMayTrap);
+  ASSERT_TRUE(CII->getRoundingMode() == ConstrainedFPIntrinsic::rmDownward);
+
+  Builder.CreateRetVoid();
+  EXPECT_FALSE(verifyModule(*M));
+}
+
 TEST_F(IRBuilderTest, Lifetime) {
   IRBuilder<> Builder(BB);
   AllocaInst *Var1 = Builder.CreateAlloca(Builder.getInt8Ty());
Index: include/llvm/IR/IntrinsicInst.h
===
--- include/llvm/IR/IntrinsicInst.h
+++ include/llvm/IR/IntrinsicInst.h
@@ -208,20 +208,30 @@
   /// This is the common base class for constrained floating point intrinsics.
   class ConstrainedFPIntrinsic : public IntrinsicInst {
   public:
-enum RoundingMode {
-  rmInvalid,
-  rmDynamic,
-  rmToNearest,
-  rmDownward,
-  rmUpward,
-  rmTowardZero
+/// Specifies the rounding mode to be assumed. This is only used when
+/// when constrained floating point is enabled. See the LLVM Language
+/// Reference Manual for details.
+enum RoundingMode : uint8_t {
+  rmUnspecified = 0, ///< Use as a placeholder to not affect rounding
+ ///< behavior in the IRBuilder.
+  rmInvalid = 0,
+  rmDynamic, ///< This corresponds to "fpround.dynamic".
+  rmToNearest,   ///< This corresponds to "fpround.tonearest".
+  rmDownward,///< This corresponds to "fpround.downward".
+  rmUpward,  ///< This corresponds to "fpround.upward".
+  rmTowardZero   ///< This corresponds to "fpround.tozero".
 };
 
-enum ExceptionBehavior {
-  ebInvalid,
-  ebIgnore,
-  ebMayTrap,
-  ebStrict
+/// Specifies the required exception behavior. This is only used when
+/// when constrained floating point is used. See the LLVM Language
+/// Reference Manual for details.
+enum ExceptionBehavior : uint8_t {
+  ebUnspecified = 0, ///< Use as a placeholder to not affect exception
+ ///< behavior in the IRBuilder.
+  ebInvalid = 0,
+  ebIgnore,  ///< This corresponds to "fpexcept.ignore".
+  ebMayTrap, ///< This corresponds to "fpexcept.maytrap".
+  ebStrict   ///< This corresponds to "fpexcept.strict".
 };
 
 bool isUnaryOp() const;
Index: 

[PATCH] D63438: [analyzer] print() JSONify: ProgramPoint revision

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Yes, this would be helpful! Tests?


Repository:
  rC Clang

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

https://reviews.llvm.org/D63438



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

r363606 - PR42205: DebugInfio: Do not attempt to emit debug info metadata for static member variable template partial specializations

[David Blaikie via cfe-commits]

Author: dblaikie
Date: Mon Jun 17 12:40:52 2019
New Revision: 363606

URL: http://llvm.org/viewvc/llvm-project?rev=363606=rev
Log:
PR42205: DebugInfio: Do not attempt to emit debug info metadata for static 
member variable template partial specializations

Would cause a crash in an attempt to create the type for the still
unresolved 'auto' in the partial specialization (& even without the use
of 'auto', the expression would be value dependent &
crash/assertion-fail there).

Added:
cfe/trunk/test/CodeGenCXX/debug-info-var-template-partial-spec.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=363606=363605=363606=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Jun 17 12:40:52 2019
@@ -1410,6 +1410,9 @@ void CGDebugInfo::CollectRecordFields(
 isa(V))
   continue;
 
+if (isa(V))
+  continue;
+
 // Reuse the existing static member declaration if one exists
 auto MI = StaticDataMemberCache.find(V->getCanonicalDecl());
 if (MI != StaticDataMemberCache.end()) {

Added: cfe/trunk/test/CodeGenCXX/debug-info-var-template-partial-spec.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-var-template-partial-spec.cpp?rev=363606=auto
==
--- cfe/trunk/test/CodeGenCXX/debug-info-var-template-partial-spec.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/debug-info-var-template-partial-spec.cpp Mon Jun 
17 12:40:52 2019
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -std=c++14 -debug-info-kind=limited -emit-llvm -o - | 
FileCheck %s
+
+// CHECK: ![[empty:[0-9]+]] = !{}
+
+// CHECK: distinct !DICompositeType(tag: DW_TAG_structure_type, name: "B",
+// CHECK-SAME: elements: ![[empty]]
+
+struct B {
+  template 
+  static const int d = 0;
+  template 
+  static const auto d = d;
+} c;


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

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:13345-13346
+<< (FD->getStorageClass() == SC_None
+? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(),
+ "static ")
+: FixItHint{});

aaronpuchert wrote:
> aaron.ballman wrote:
> > We may not want to produce the fixit if there's a macro involved in the 
> > declaration. Consider:
> > ```
> > #ifdef SOMETHING
> > #define FROBBLE static
> > #else
> > #define FROBBLE
> > #endif
> > 
> > FROBBLE void foo(void);
> > ```
> > We probably don't want the fixit in the case `SOMETHING` is not defined.
> I think that's generally an issue with fix-its, there could always be a macro 
> that turns the code into something entirely different. If we look at the 
> other fix-it above, we can construct
> 
> ```
> #define VOID
> int f(VOID);
> int f() { return 0; }
> ```
> 
> Then we get:
> ```
> :3:5: warning: no previous prototype for function 'f' 
> [-Wmissing-prototypes]
> int f() { return 0; }
> ^
> :2:5: note: this declaration is not a prototype; add 'void' to make it 
> a prototype for a zero-parameter function
> int f(VOID);
> ^
>   void
> ```
> 
> Then the fix-it doesn't even work in the original configuration, because it 
> produces `int f(VOIDvoid)`. If we make it work by adding a space, we still 
> have the problem that you mentioned: if someone defines `VOID` as `void`, we 
> then have `int f(void void)` after applying the fix-it in the original 
> setting.
> 
> Trying to make fix-its work with macros is probably a hopeless endeavor.
> Trying to make fix-its work with macros is probably a hopeless endeavor.

That's what I was getting at -- I think you need to see if there's a macro at 
the insertion location and not generate a fix-it in that case. The above 
suffers from the same issue (and can be corrected in a subsequent patch, if 
desired).


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

[PATCH] D63367: [clang-doc] Add basic support for templates and typedef

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Representation.h:244
  // interface).
+  bool IsTypeDef = false; // Indicates if record was declared using typedef
   llvm::SmallVector

Run clang-format? Not totally sure what it'll do, but I think it should line up 
the comments.



Comment at: clang-tools-extra/clang-doc/Serialize.cpp:387
 
-  SymbolID ParentUSR = getUSRForDecl(D->getParent());
-  Func.Parent = Reference{ParentUSR, D->getParent()->getNameAsString(),
-  InfoType::IT_record};
+  const NamedDecl *Parent;
+  if (const auto *SD =

Generally, it's not a great idea to declare a pointer without initializing it. 
Either initialize it on declaration or initialize to nullptr.


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

https://reviews.llvm.org/D63367



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

[PATCH] D59402: Suggestions to fix -Wmissing-{prototypes,variable-declarations}

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:13345-13346
+<< (FD->getStorageClass() == SC_None
+? FixItHint::CreateInsertion(FD->getTypeSpecStartLoc(),
+ "static ")
+: FixItHint{});

aaron.ballman wrote:
> We may not want to produce the fixit if there's a macro involved in the 
> declaration. Consider:
> ```
> #ifdef SOMETHING
> #define FROBBLE static
> #else
> #define FROBBLE
> #endif
> 
> FROBBLE void foo(void);
> ```
> We probably don't want the fixit in the case `SOMETHING` is not defined.
I think that's generally an issue with fix-its, there could always be a macro 
that turns the code into something entirely different. If we look at the other 
fix-it above, we can construct

```
#define VOID
int f(VOID);
int f() { return 0; }
```

Then we get:
```
:3:5: warning: no previous prototype for function 'f' 
[-Wmissing-prototypes]
int f() { return 0; }
^
:2:5: note: this declaration is not a prototype; add 'void' to make it a 
prototype for a zero-parameter function
int f(VOID);
^
  void
```

Then the fix-it doesn't even work in the original configuration, because it 
produces `int f(VOIDvoid)`. If we make it work by adding a space, we still have 
the problem that you mentioned: if someone defines `VOID` as `void`, we then 
have `int f(void void)` after applying the fix-it in the original setting.

Trying to make fix-its work with macros is probably a hopeless endeavor.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59402



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

[PATCH] D63260: [Attr] Support _attribute__ ((fallthrough))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/AnalysisBasedWarnings.cpp:1279
 continue;
-  if (S.getLangOpts().CPlusPlus11) {
+  if (S.getLangOpts().CPlusPlus11 || S.getLangOpts().C99) {
 const Stmt *Term = B->getTerminatorStmt();

xbolva00 wrote:
> nickdesaulniers wrote:
> > xbolva00 wrote:
> > > nickdesaulniers wrote:
> > > > Probably should additionally/instead check `S.getLangOpts().GNUMode` 
> > > > (since these are GNU C style attributes)?  I guess we want these 
> > > > attributes to be supported in C regardless of `-std=`?
> > > Good point.
> > IIUC, we allow GNU C attributes for C regardless of ISO vs GNU C.  What we 
> > want to express is "if c++11 and newer, or c".
> > 
> > I think this might be better expressed as:
> > 
> > ```
> > if (S.getLangOpts().CPlusPlus11 || (!S.getLangOpts().CPlusPlus && 
> > !S.getLangOpts().ObjC) {
> > ```
> > 
> > But maybe there's a more canonical way to express "if the language is C." 
> > (If not, maybe we can consider such a change to `LangOpt` to make this 
> > easier, but as part of this commit).
> Yeah, I wanted to check if lang is C but I didnt find a nice way :(
I think we want to support this attribute in ObjC because that language is 
based on C. In fact, I think we are supporting this attribute in all language 
modes regardless of -std level because the user can always use 
`__attribute__((fallthrough))` as a spelling.



Comment at: lib/Sema/AnalysisBasedWarnings.cpp:1233
+else
+  MacroName = "__attribute__ ((fallthrough))";
+  }

I'd prefer to see this be `__attribute__((fallthrough))` without the extra 
whitespace.


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

https://reviews.llvm.org/D63260



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

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:73
+/// Incremental builder for an llvm::Constant* holding a structure constant.
+class ConstantBuilder : private ConstantBuilderUtils {
+  llvm::SmallVector Elems;

This seems like a very generic name for this type.



Comment at: lib/CodeGen/CGExprConstant.cpp:75
+  llvm::SmallVector Elems;
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();

Are there invariants about these?  I assume they're parallel arrays; are they 
kept sorted?



Comment at: lib/CodeGen/CGExprConstant.cpp:76
+  llvm::SmallVector Offsets;
+  CharUnits Size = CharUnits::Zero();
+

This is one past the last byte that's been covered by an actual `Constant*` 
value, or does it include unoccupied padding, or does it exclude even occupied 
padding?



Comment at: lib/CodeGen/CGExprConstant.cpp:98
+Offsets.reserve(N);
+  }
+

Might be worth clarifying what `N` is here.



Comment at: lib/CodeGen/CGExprConstant.cpp:190
+bool ConstantBuilder::addBits(llvm::APInt Bits, uint64_t OffsetInBits,
+  bool AllowOverwrite) {
+  const ASTContext  = CGM.getContext();

`AllowOversized` (which you used in the interface) seems like a better name.



Comment at: lib/CodeGen/CGExprConstant.cpp:196
+  // current char.
+  unsigned CharOffset = OffsetInBits % CharWidth;
+

`OffsetWithinChar`?



Comment at: lib/CodeGen/CGExprConstant.cpp:201
+  for (CharUnits Char = Context.toCharUnitsFromBits(OffsetInBits);
+   /**/; ++Char) {
+// Number of bits we want to fill in this byte.

`OffsetInChars`?



Comment at: lib/CodeGen/CGExprConstant.cpp:237
+  if (!LastElemToUpdate)
+return false;
+  assert(*LastElemToUpdate - *FirstElemToUpdate < 2 &&

Especially in the context of the comment above, I think it would be good to 
clarify that both of these are hard "we can't emit this constant" bail-outs.



Comment at: lib/CodeGen/CGExprConstant.cpp:258
+  return false;
+assert(CI->getBitWidth() == CharWidth && "splitAt failed");
+assert((!(CI->getValue() & UpdateMask) || AllowOverwrite) &&

Oh, because we're splitting before and after a single-`CharUnits` range?  That 
seems worthy of a somewhat clearer explanation in the code.

I guess we could have a non-`ConstantInt` single-byte value.  Unlikely but not 
impossible. :)



Comment at: lib/CodeGen/CGExprConstant.cpp:375
+
+  // FIXME: We could theoretically split a ConstantInt if the need ever arose.
+

Does this not come up all the time with bit-fields?  I guess we emit them in 
single-`char` chunks, so it wouldn't.  Probably worth a comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:13335
+? FixItHint::CreateInsertion(FTL.getRParenLoc(), 
"void")
+: FixItHint{});
 }

aaronpuchert wrote:
> aaron.ballman wrote:
> > In this case, we could probably generate the parameter declarations for the 
> > user still, couldn't we? e.g.,
> > ```
> > int f(); // We could insert "int i, int j" here as the fixit
> > int g(); // Just like we try to suggest "void" here already
> > 
> > int f(int i, int j) { return i + j; }
> > int g(void) { return 12; }
> > ```
> > That could be done in a follow-up patch though.
> One difficulty might be that certain types used in the parameter list of the 
> definition aren't available where the declaration is, like
> 
> ```
> int f();
> 
> struct X { int n; };
> int f(struct X *x) { return x->n;}
> ```
> 
> So I'm not sure we can always do it, but sometimes perhaps.
That's a good point and reason enough to not try for it in this version of the 
patch (if at all).


Repository:
  rC Clang

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

https://reviews.llvm.org/D62750



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

[PATCH] D62750: Show note for -Wmissing-prototypes for functions with parameters

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:13335
+? FixItHint::CreateInsertion(FTL.getRParenLoc(), 
"void")
+: FixItHint{});
 }

aaron.ballman wrote:
> In this case, we could probably generate the parameter declarations for the 
> user still, couldn't we? e.g.,
> ```
> int f(); // We could insert "int i, int j" here as the fixit
> int g(); // Just like we try to suggest "void" here already
> 
> int f(int i, int j) { return i + j; }
> int g(void) { return 12; }
> ```
> That could be done in a follow-up patch though.
One difficulty might be that certain types used in the parameter list of the 
definition aren't available where the declaration is, like

```
int f();

struct X { int n; };
int f(struct X *x) { return x->n;}
```

So I'm not sure we can always do it, but sometimes perhaps.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62750



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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

[John Regehr via Phabricator via cfe-commits]

regehr added a comment.

Just wanted to say that I think I agree with the design choices here!


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

https://reviews.llvm.org/D63423



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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

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



Comment at: test/SemaCXX/warn-xor-as-pow.cpp:85
+  res = XOR(10, 10);
+  res = 10^-7;
+}

What should we suggest here ? :) Nevative shift ? think we should diagnose it.

I am not sure about 10 ^ 0 case... warn or not ?

@jfb 


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

https://reviews.llvm.org/D63423



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

[PATCH] D63376: [clang] Small improvments after Adding APValue to ConstantExpr

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

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

Nice cleanup!




Comment at: clang/include/clang/Sema/Sema.h:10298
+ bool AllowFold = true,
+ bool StoreResult = true);
   ExprResult VerifyIntegerConstantExpression(Expr *E, llvm::APSInt *Result,

Do you need this new flag? No callers are passing it.


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

https://reviews.llvm.org/D63376



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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

xbolva00 updated this revision to Diff 205140.
xbolva00 added a comment.

Removed useless else block with return.


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define FOOBAR(x, y) (x * y)
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+#define EPSILON 10^-300
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+  res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0, maybe you mean '1<<2' (4)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<2"
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1<<16"
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
+  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<= width of type}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL<<32"
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '0x2 ^ 64' to silence this warning}}
+
+  res = 10 ^ 0;
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"10"
+  // expected-note@-2 {{replace expression with '0xA ^ 1' to silence this warning}}
+  res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 2' to silence this warning}}
+  res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14, maybe you mean '1'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1"
+  // expected-note@-2 {{replace expression with '0xA ^ 4' to silence this warning}}
+  res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = 0xA ^ 10;
+  #if defined(KW_XOR)
+  res = 10 xor 10;
+  #endif
+  res = XOR(10, 10);
+  res = 10^-7;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10890,6 +10890,86 @@
   return GetSignedVectorType(vType);
 }
 
+static void diagnoseXorMisusedAsPow(Sema , ExprResult , ExprResult ,
+SourceLocation Loc) {
+  // Do not diagnose macros
+ 

[PATCH] D63371: Rewrite ConstStructBuilder with a mechanism that can cope with splitting and updating constants.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D63371#1546587 , @rsmith wrote:

> In D63371#1546500 , @rjmccall wrote:
>
> > Isn't `[[no_unique_address]]` only significant for empty members?  I'm not 
> > sure why they need significant support from constant-building, since they 
> > expand to no meaningful initializer.
>
>
> It also permits reuse of tail padding for non-static data members, which is 
> the complexity that this patch is dealing with (in addition to improving and 
> generalizing the support for non-trivial designated initializers).


I see.

>> We have some code we'll hopefully be upstreaming soon that relies on being 
>> able to do things with address-of-position placeholder values; I'm a little 
>> worried that the new structure here doesn't really support them.
> 
> Can you say a bit more about that? (Do you want to be able to emit a constant 
> that denotes a pointer to somewhere else within the same constant being 
> emitted, or something like that?) I think this approach should be strictly 
> more general than what we had before, but perhaps that means it can't be 
> extended in the direction you need?

We need to be able to construct constant initializers for certain fields in 
terms of (among other things) a pointer to the current field.  My concern was 
whether this might mess up the indexing to the current field; but it looks like 
we actually discover the right GEP indices retroactively for these aggregate 
constants (unlike e.g. `ConstantInitBuilder`), so the fact that the GEP indices 
might change as we build the constant is not a problem.  As long as the 
`ConstantEmitter` is being passed around and used to build the individual 
fields appropriately, it should be fine.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63371



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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

xbolva00 updated this revision to Diff 205136.
xbolva00 added a comment.

Updated.


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

https://reviews.llvm.org/D63423

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify -Wxor-as-pow %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DKW_XOR -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+
+#define FOOBAR(x, y) (x * y)
+#define XOR(x, y) (x ^ y)
+#define TWO 2
+#define TEN 10
+#define TWO_ULL 2ULL
+#define EPSILON 10^-300
+
+void test(unsigned a, unsigned b) {
+  unsigned res;
+  res = a ^ 5;
+  res = 2 ^ b;
+  res = a ^ b;
+  res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2, maybe you mean '1<<0' (1)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+  res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3, maybe you mean '1<<1' (2)?}}
+   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<1"
+   // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+  res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0, maybe you mean '1<<2' (4)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<2"
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10, maybe you mean '1<<8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1<<8"
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18, maybe you mean '1<<16' (65536)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1<<16"
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
+  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8, maybe you mean '1<= width of type}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL<<32"
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
+  res = 2 ^ 64; // expected-warning {{result of '2 ^ 64' is 66; maybe you mean '1<<64', but shift count >= width of type}}
+  // expected-note@-1 {{replace expression with '0x2 ^ 64' to silence this warning}}
+
+  res = 10 ^ 0;
+  res = 10 ^ 1; // expected-warning {{result of '10 ^ 1' is 11, maybe you mean '10'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"10"
+  // expected-note@-2 {{replace expression with '0xA ^ 1' to silence this warning}}
+  res = 10 ^ 2; // expected-warning {{result of '10 ^ 2' is 8, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 2' to silence this warning}}
+  res = 10 ^ 4; // expected-warning {{result of '10 ^ 4' is 14, maybe you mean '1'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1"
+  // expected-note@-2 {{replace expression with '0xA ^ 4' to silence this warning}}
+  res = 10 ^ 10; // expected-warning {{result of '10 ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = TEN ^ 10; // expected-warning {{result of 'TEN ^ 10' is 0, maybe you mean '100'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:17}:"100"
+  // expected-note@-2 {{replace expression with '0xA ^ 10' to silence this warning}}
+  res = 0xA ^ 10;
+  #if defined(KW_XOR)
+  res = 10 xor 10;
+  #endif
+  res = XOR(10, 10);
+  res = 10^-7;
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10890,6 +10890,89 @@
   return GetSignedVectorType(vType);
 }
 
+static void diagnoseXorMisusedAsPow(Sema , ExprResult , ExprResult ,
+SourceLocation Loc) {
+  // Do not diagnose macros
+  if (Loc.isMacroID())
+

[PATCH] D58033: Add option for emitting dbg info for call site parameters

[Paul Robinson via Phabricator via cfe-commits]

probinson added inline comments.



Comment at: lib/Frontend/CompilerInvocation.cpp:755
+  (Arch == llvm::Triple::x86 || Arch == llvm::Triple::x86_64))
+Opts.EnableDebugEntryValues = Args.hasArg(OPT_femit_debug_entry_values);
+

You want to disable entry-values for all targets other than X86?


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

https://reviews.llvm.org/D58033



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

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

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

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



Comment at: lib/Sema/SemaExpr.cpp:10924
+
+  if (LeftSideValue == 2 || LeftSideValue == 10) {
+llvm::APInt XorValue = LeftSideValue ^ RightSideValue;

Quuxplusone wrote:
> Do you have metrics indicating that this line is an improvement over `if 
> (true) {`?
This is left over of older code :) I will fix it.



Comment at: test/Sema/warn-xor-as-pow.c:39
+  res = 2 ^ 0b100;
+  res = XOR(2, 16);
+  unsigned char two = 2;

Quuxplusone wrote:
> I don't understand why this line doesn't warn. Is it because the macro's name 
> has the case-insensitive string `xor` in it? Is it because there is a macro 
> involved at all? In either case, please add another test case for `res = 
> FOOBAR(2, 16)`.
> 
> Also `res = EPSILON` where `#define EPSILON 10^-300`. That seems to come up 
> in the codesearch results a lot.
EPSILON 10^-300

this is in macro :( 

so maybe if negative RHS, we should check macro too? @jfb 


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

https://reviews.llvm.org/D63423



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

[PATCH] D63451: P0840R2: support for [[no_unique_address]] attribute

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

rsmith created this revision.
rsmith added a reviewer: rjmccall.
Herald added subscribers: aheejin, dschuff.
Herald added a project: clang.

Add support for the C++2a [[no_unique_address]] attribute for targets using the 
Itanium C++ ABI.

This depends on D63371 .


Repository:
  rC Clang

https://reviews.llvm.org/D63451

Files:
  include/clang/AST/Decl.h
  include/clang/AST/DeclCXX.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/AST/Decl.cpp
  lib/AST/DeclCXX.cpp
  lib/AST/RecordLayoutBuilder.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGRecordLayoutBuilder.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGenCXX/no-unique-address.cpp
  test/CodeGenCXX/tail-padding.cpp
  test/Layout/no-unique-address.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -2810,7 +2810,7 @@
 
 // Helper function for GenerateTargetSpecificAttrChecks that alters the 'Test'
 // parameter with only a single check type, if applicable.
-static void GenerateTargetSpecificAttrCheck(const Record *R, std::string ,
+static bool GenerateTargetSpecificAttrCheck(const Record *R, std::string ,
 std::string *FnName,
 StringRef ListName,
 StringRef CheckAgainst,
@@ -2830,7 +2830,9 @@
 *FnName += Part;
 }
 Test += ")";
+return true;
   }
+  return false;
 }
 
 // Generate a conditional expression to check if the current target satisfies
@@ -2838,10 +2840,12 @@
 // those checks to the Test string. If the FnName string pointer is non-null,
 // append a unique suffix to distinguish this set of target checks from other
 // TargetSpecificAttr records.
-static void GenerateTargetSpecificAttrChecks(const Record *R,
+static bool GenerateTargetSpecificAttrChecks(const Record *R,
  std::vector ,
  std::string ,
  std::string *FnName) {
+  bool AnyTargetChecks = false;
+
   // It is assumed that there will be an llvm::Triple object
   // named "T" and a TargetInfo object named "Target" within
   // scope that can be used to determine whether the attribute exists in
@@ -2851,6 +2855,7 @@
   // differently because GenerateTargetRequirements needs to combine the list
   // with ParseKind.
   if (!Arches.empty()) {
+AnyTargetChecks = true;
 Test += " && (";
 for (auto I = Arches.begin(), E = Arches.end(); I != E; ++I) {
   StringRef Part = *I;
@@ -2865,16 +2870,19 @@
   }
 
   // If the attribute is specific to particular OSes, check those.
-  GenerateTargetSpecificAttrCheck(R, Test, FnName, "OSes", "T.getOS()",
-  "llvm::Triple::");
+  AnyTargetChecks |= GenerateTargetSpecificAttrCheck(
+  R, Test, FnName, "OSes", "T.getOS()", "llvm::Triple::");
 
   // If one or more CXX ABIs are specified, check those as well.
   GenerateTargetSpecificAttrCheck(R, Test, FnName, "CXXABIs",
   "Target.getCXXABI().getKind()",
   "TargetCXXABI::");
   // If one or more object formats is specified, check those.
-  GenerateTargetSpecificAttrCheck(R, Test, FnName, "ObjectFormats",
-  "T.getObjectFormat()", "llvm::Triple::");
+  AnyTargetChecks |=
+  GenerateTargetSpecificAttrCheck(R, Test, FnName, "ObjectFormats",
+  "T.getObjectFormat()", "llvm::Triple::");
+
+  return AnyTargetChecks;
 }
 
 static void GenerateHasAttrSpellingStringSwitch(
@@ -3510,7 +3518,7 @@
 
   std::string FnName = "isTarget";
   std::string Test;
-  GenerateTargetSpecificAttrChecks(R, Arches, Test, );
+  bool UsesT = GenerateTargetSpecificAttrChecks(R, Arches, Test, );
 
   // If this code has already been generated, simply return the previous
   // instance of it.
@@ -3520,7 +3528,8 @@
 return *I;
 
   OS << "static bool " << FnName << "(const TargetInfo ) {\n";
-  OS << "  const llvm::Triple  = Target.getTriple();\n";
+  if (UsesT)
+OS << "  const llvm::Triple  = Target.getTriple();\n";
   OS << "  return " << Test << ";\n";
   OS << "}\n\n";
 
Index: test/Layout/no-unique-address.cpp
===
--- /dev/null
+++ test/Layout/no-unique-address.cpp
@@ -0,0 +1,265 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -triple x86_64-linux-gnu -fdump-record-layouts %s | FileCheck %s
+
+namespace Empty {
+  struct A {};
+  struct B { [[no_unique_address]] A a; char b; };
+  static_assert(sizeof(B) == 1);
+
+  // CHECK:*** Dumping AST 

[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added a comment.

Thanks for the patch and following up on the review comments.  Let's work on 
getting you commit access now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63369



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

[PATCH] D63369: [AST] Fixed extraneous warnings for binary conditional operator

[Nathan Huckleberry via Phabricator via cfe-commits]

Nathan-Huckleberry updated this revision to Diff 205129.
Nathan-Huckleberry added a comment.

- [AST] Formatting changes to ConditionalOperator unused detection


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63369

Files:
  clang/lib/AST/Expr.cpp
  clang/test/Sema/warn-binary-conditional-expression-unused.c


Index: clang/test/Sema/warn-binary-conditional-expression-unused.c
===
--- /dev/null
+++ clang/test/Sema/warn-binary-conditional-expression-unused.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-value -verify %s
+int main() {
+int a;
+int b;
+a ? : b; //expected-warning{{expression result unused}}
+a ? a : b; //expected-warning{{expression result unused}}
+a ? : ++b;
+a ? a : ++b;
+++a ? : b; //expected-warning{{expression result unused}}
+++a ? a : b; //expected-warning{{expression result unused}}
+++a ? : ++b;
+++a ? a : ++b;
+return 0;
+};
+
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2345,12 +2345,13 @@
 // If only one of the LHS or RHS is a warning, the operator might
 // be being used for control flow. Only warn if both the LHS and
 // RHS are warnings.
-const ConditionalOperator *Exp = cast(this);
-if (!Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx))
-  return false;
-if (!Exp->getLHS())
-  return true;
-return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+const auto *Exp = cast(this);
+return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) &&
+   Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+  }
+  case BinaryConditionalOperatorClass: {
+const auto *Exp = cast(this);
+return Exp->getFalseExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, 
Ctx);
   }
 
   case MemberExprClass:


Index: clang/test/Sema/warn-binary-conditional-expression-unused.c
===
--- /dev/null
+++ clang/test/Sema/warn-binary-conditional-expression-unused.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-value -verify %s
+int main() {
+int a;
+int b;
+a ? : b; //expected-warning{{expression result unused}}
+a ? a : b; //expected-warning{{expression result unused}}
+a ? : ++b;
+a ? a : ++b;
+++a ? : b; //expected-warning{{expression result unused}}
+++a ? a : b; //expected-warning{{expression result unused}}
+++a ? : ++b;
+++a ? a : ++b;
+return 0;
+};
+
Index: clang/lib/AST/Expr.cpp
===
--- clang/lib/AST/Expr.cpp
+++ clang/lib/AST/Expr.cpp
@@ -2345,12 +2345,13 @@
 // If only one of the LHS or RHS is a warning, the operator might
 // be being used for control flow. Only warn if both the LHS and
 // RHS are warnings.
-const ConditionalOperator *Exp = cast(this);
-if (!Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx))
-  return false;
-if (!Exp->getLHS())
-  return true;
-return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+const auto *Exp = cast(this);
+return Exp->getLHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx) &&
+   Exp->getRHS()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
+  }
+  case BinaryConditionalOperatorClass: {
+const auto *Exp = cast(this);
+return Exp->getFalseExpr()->isUnusedResultAWarning(WarnE, Loc, R1, R2, Ctx);
   }
 
   case MemberExprClass:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits