[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 455485.
inclyc added a comment.

Fix CI issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/FormatString.h
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/FixIt/format.m
  clang/test/FixIt/format.mm
  clang/test/Sema/format-strings-freebsd.c
  clang/test/Sema/format-strings-scanf.c
  clang/test/Sema/format-strings.c
  clang/test/SemaObjC/format-strings-objc.m
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -819,13 +819,7 @@
 
   Unclear type relationship between a format specifier and its argument
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf";>N2562
-  
-Partial
-  Clang supports diagnostics checking format specifier validity, but
-  does not yet account for all of the changes in this paper, especially
-  regarding length modifiers like h and hh.
-
-  
+  Clang 16
 
 
 
Index: clang/test/SemaObjC/format-strings-objc.m
===
--- clang/test/SemaObjC/format-strings-objc.m
+++ clang/test/SemaObjC/format-strings-objc.m
@@ -80,7 +80,7 @@
 
 //  - Catch use of long long with int arguments.
 void rdar_7068334(void) {
-  long long test = 500;  
+  long long test = 500;
   printf("%i ",test); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
   NSLog(@"%i ",test); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
   CFStringCreateWithFormat(CFSTR("%i"),test); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
@@ -191,7 +191,7 @@
   NSLog(@"%C", data);  // no-warning
 
   const wchar_t wchar_data = L'a';
-  NSLog(@"%C", wchar_data);  // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
+  NSLog(@"%C", wchar_data);  // no-warning
 }
 
 // Test that %@ works with toll-free bridging ().
@@ -273,7 +273,7 @@
 
 void testUnicode(void) {
   NSLog(@"%C", 0x2022); // no-warning
-  NSLog(@"%C", 0x202200); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
+  NSLog(@"%C", 0x202200); // no-warning
 }
 
 // Test Objective-C modifier flags.
Index: clang/test/Sema/format-strings.c
===
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -830,3 +830,57 @@
   printf_arg2("foo", "%s string %i\n", "aaa", 123);
   printf_arg2("%s string\n", "foo", "bar"); // expected-warning{{data argument not used by format string}}
 }
+
+void test_promotion(void) {
+  // Default argument promotions for *printf in N2562
+  // https://github.com/llvm/llvm-project/issues/57102
+  // N2562: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
+  int i;
+  signed char sc;
+  unsigned char uc;
+  char c;
+  short ss;
+  unsigned short us;
+
+  printf("%hhd %hd %d %hhd %hd %d", i, i, i, sc, sc, sc); // no-warning
+  printf("%hhd %hd %d %hhd %hd %d", uc, uc, uc, c, c, c); // no-warning
+
+  // %ld %lld %llx
+  printf("%ld", i); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
+  printf("%lld", i); // expected-warning{{format specifies type 'long long' but the argument has type 'int'}}
+  printf("%ld", sc); // expected-warning{{format specifies type 'long' but the argument has type 'signed char'}}
+  printf("%lld", sc); // expected-warning{{format specifies type 'long long' but the argument has type 'signed char'}}
+  printf("%ld", uc); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned char'}}
+  printf("%lld", uc); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned char'}}
+  printf("%llx", i); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'int'}}
+
+  // ill formed spec for floats
+  printf("%hf", // expected-warning{{length modifier 'h' results in undefined behavior or no effect with 'f' conversion specifier}}
+  sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+
+  // for %hhd and `short` they are compatible by promotions but more likely misuse
+  printf("%hd", ss); // no-warning
+  printf("%hhd", ss); // expected-warning{{format specifies type 'char' but the argument has type 'short'}}
+  printf("%hu", us); // no-warning
+  printf("%hhu", ss); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'short'}}
+
+  // floats & integers are not compatible
+  printf("%f", i); // expected-warning{

[PATCH] D127082: [clangd] Add Macro Expansion to Hover

[Qingyuan Zheng via Phabricator via cfe-commits]

daiyousei-qz added a comment.

@sammccall Hello, is there any update required by me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127082

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

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

[Michael Buch via Phabricator via cfe-commits]

Michael137 added a comment.

In D128830#3746153 , @aprantl wrote:

> I think we can "fix" the test with the following patch:
>
>   diff --git a/lldb/test/API/functionalities/unused-inlined-parameters/main.c 
> b/lldb/test/API/functionalities/unused-inlined-parameters/main.c
>   index f2ef5dcc213d..9b9f95f6c946 100644
>   --- a/lldb/test/API/functionalities/unused-inlined-parameters/main.c
>   +++ b/lldb/test/API/functionalities/unused-inlined-parameters/main.c
>   @@ -7,6 +7,7 @@ __attribute__((always_inline)) void f(void *unused1, int 
> used, int unused2) {
>}
>
>int main(int argc, char **argv) {
>   -  f(argv, 42, 1);
>   +  char *undefined;
>   +  f(undefined, 42, 1);
>  return 0;
>   -}

Made the change to the test. Confirmed it passes with and without the patch. 
Feel free to push again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added inline comments.



Comment at: clang/tools/libclang/CMakeLists.txt:9-12
+else()
+  # ... unless explicily overridden
+  set(LIBCLANG_SOVERSION ${CLANG_VERSION_MAJOR})
+endif()

Sorry I didn't get to comment in time, but now that the default was switched 
from OFF to ON, the comments here are lying...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132486

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

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: clang/tools/libclang/CMakeLists.txt:9-12
+else()
+  # ... unless explicily overridden
+  set(LIBCLANG_SOVERSION ${CLANG_VERSION_MAJOR})
+endif()

h-vetinari wrote:
> Sorry I didn't get to comment in time, but now that the default was switched 
> from OFF to ON, the comments here are lying...
Thanks - will fix it in main as a NFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132486

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

[clang] 879f511 - [Pipelines] Introduce DAE after ArgumentPromotion

[Pavel Samolysov via cfe-commits]

Author: Pavel Samolysov
Date: 2022-08-25T10:55:47+03:00
New Revision: 879f5118fc74657e4a5c4eff6810098e1eed75ac

URL: 
https://github.com/llvm/llvm-project/commit/879f5118fc74657e4a5c4eff6810098e1eed75ac
DIFF: 
https://github.com/llvm/llvm-project/commit/879f5118fc74657e4a5c4eff6810098e1eed75ac.diff

LOG: [Pipelines] Introduce DAE after ArgumentPromotion

The ArgumentPromotion pass uses Mem2Reg promotion at the end to cutting
down generated `alloca` instructions as well as meaningless `store`s and
this behavior can leave unused (dead) arguments. To eliminate the dead
arguments and therefore let the DeadCodeElimination remove becoming dead
inserted `GEP`s as well as `load`s and `cast`s in the callers, the
DeadArgumentElimination pass should be run after the ArgumentPromotion
one.

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

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-newpm.ll
llvm/lib/Passes/PassBuilderPipelines.cpp
llvm/test/Other/new-pm-defaults.ll
llvm/test/Other/new-pm-lto-defaults.ll
llvm/test/Other/new-pm-thinlto-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
llvm/test/Transforms/InstCombine/unused-nonnull.ll
llvm/test/Transforms/PhaseOrdering/dce-after-argument-promotion.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-newpm.ll 
b/clang/test/CodeGen/thinlto-distributed-newpm.ll
index 463fc522c6a28..64ac49420cbdc 100644
--- a/clang/test/CodeGen/thinlto-distributed-newpm.ll
+++ b/clang/test/CodeGen/thinlto-distributed-newpm.ll
@@ -34,7 +34,6 @@
 ; CHECK-O: Running pass: CalledValuePropagationPass
 ; CHECK-O: Running pass: GlobalOptPass
 ; CHECK-O: Running pass: PromotePass
-; CHECK-O: Running pass: DeadArgumentEliminationPass
 ; CHECK-O: Running pass: InstCombinePass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main
 ; CHECK-O: Running pass: InlinerPass on (main)
@@ -74,6 +73,7 @@
 ; CHECK-O: Running pass: LCSSAPass on main
 ; CHECK-O: Running pass: SimplifyCFGPass on main
 ; CHECK-O: Running pass: InstCombinePass on main
+; CHECK-O: Running pass: DeadArgumentEliminationPass
 ; CHECK-O: Running pass: GlobalOptPass
 ; CHECK-O: Running pass: GlobalDCEPass
 ; CHECK-O: Running pass: EliminateAvailableExternallyPass

diff  --git a/llvm/lib/Passes/PassBuilderPipelines.cpp 
b/llvm/lib/Passes/PassBuilderPipelines.cpp
index bd07638b37617..2587c8ce900b9 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -639,7 +639,7 @@ void PassBuilder::addPGOInstrPasses(ModulePassManager &MPM,
 
 FunctionPassManager FPM;
 FPM.addPass(SROAPass());
-FPM.addPass(EarlyCSEPass());// Catch trivial redundancies.
+FPM.addPass(EarlyCSEPass()); // Catch trivial redundancies.
 FPM.addPass(SimplifyCFGPass(SimplifyCFGOptions().convertSwitchRangeToICmp(
 true)));// Merge & remove basic blocks.
 FPM.addPass(InstCombinePass()); // Combine silly sequences.
@@ -734,10 +734,9 @@ PassBuilder::buildInlinerPipeline(OptimizationLevel Level,
   if (PGOOpt)
 IP.EnableDeferral = EnablePGOInlineDeferral;
 
-  ModuleInlinerWrapperPass MIWP(
-  IP, PerformMandatoryInliningsFirst,
-  InlineContext{Phase, InlinePass::CGSCCInliner},
-  UseInlineAdvisor, MaxDevirtIterations);
+  ModuleInlinerWrapperPass MIWP(IP, PerformMandatoryInliningsFirst,
+InlineContext{Phase, InlinePass::CGSCCInliner},
+UseInlineAdvisor, MaxDevirtIterations);
 
   // Require the GlobalsAA analysis for the module so we can query it within
   // the CGSCC pipeline.
@@ -961,10 +960,6 @@ 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   // constants.
   MPM.addPass(createModuleToFunctionPassAdaptor(PromotePass()));
 
-  // Remove any dead arguments exposed by cleanups and constant folding
-  // globals.
-  MPM.addPass(DeadArgumentEliminationPass());
-
   // Create a small function pass pipeline to cleanup after all the global
   // optimizations.
   FunctionPassManager GlobalCleanupPM;
@@ -999,6 +994,10 @@ 
PassBuilder::buildModuleSimplificationPipeline(OptimizationLevel Level,
   else
 MPM.addPass(buildInlinerPipeline(Level, Phase));
 
+  // Remove any dead arguments exposed by cleanups, constant folding globals,
+  // and argument promotion.
+  MPM.addPass(DeadArgumentEliminationPass());
+
   MPM.addPass(CoroCleanupPass());
 
   if (EnableMemProfiler && Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
@@ -1596,9 +1595,6 @@ PassBuilder::buildLTODefaultPipeline(OptimizationLevel 
Level,
   // keep one copy of each constant.
   MPM.addPass(ConstantMergePass());
 
-  // Remove unused arguments fro

[clang] 5218a54 - [NFC] Fix a misleading comment CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[Tobias Hieta via cfe-commits]

Author: Tobias Hieta
Date: 2022-08-25T09:56:55+02:00
New Revision: 5218a542ac09ea97f78681f4f1b90a3b835c

URL: 
https://github.com/llvm/llvm-project/commit/5218a542ac09ea97f78681f4f1b90a3b835c
DIFF: 
https://github.com/llvm/llvm-project/commit/5218a542ac09ea97f78681f4f1b90a3b835c.diff

LOG: [NFC] Fix a misleading comment CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

Added: 


Modified: 
clang/tools/libclang/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/libclang/CMakeLists.txt 
b/clang/tools/libclang/CMakeLists.txt
index 9a874ae6f85dc..c6b3b44a76334 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -3,11 +3,15 @@
 # LLVM_VERSION_MAJOR.
 # Please also see clang/tools/libclang/libclang.map
 
+# This option defaults to CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION
+# to ON - which means that it by default matches CLANG_VERSION_MAJOR
+#
+# TODO: This should probably not be a option going forward but we
+# we should commit to a way to do it. But due to getting this out
+# in LLVM 15.x we opted for a option.
 if(NOT CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION)
-  # default is to use the SOVERSION according to ABI...
   set(LIBCLANG_SOVERSION 13)
 else()
-  # ... unless explicily overridden
   set(LIBCLANG_SOVERSION ${CLANG_VERSION_MAJOR})
 endif()
 



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

[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

[Pavel Samolysov via Phabricator via cfe-commits]

psamolysov added a comment.

Colleagues, thank you for the discussion.

@aprantl @Michael137 Thank you very much for the patch and the test changes and 
confirmation. I've pushed the patch again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added a comment.

I have another question, probably mainly for @tstellar (since I don't 
understand the `clang/tools/libclang/libclang.{exports,map}` infrastructure). 
Now that we're defaulting back to the equality case, would we need to reinstate 
`libclang.exports`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132486

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

[clang-tools-extra] ab9ee47 - [clang-tidy] Add test to cert-dcl58-cpp (NFC).

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2022-08-25T10:11:40+02:00
New Revision: ab9ee471dfda27b3cfb4a350e406576512ce9053

URL: 
https://github.com/llvm/llvm-project/commit/ab9ee471dfda27b3cfb4a350e406576512ce9053
DIFF: 
https://github.com/llvm/llvm-project/commit/ab9ee471dfda27b3cfb4a350e406576512ce9053.diff

LOG: [clang-tidy] Add test to cert-dcl58-cpp (NFC).

Add a test to cert-dcl58-cpp.cpp to verify that crash in github
issue #56902 does not happen.

Reviewed By: njames93

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

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp

Removed: 




diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp
index 79beae12792ff..01964e7dc6c76 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp
@@ -268,3 +268,16 @@ struct numeric_limits<::ranges::detail::LongT> {
 inline constexpr bool numeric_limits<::ranges::detail::LongT>::is_signed;
 inline constexpr bool numeric_limits<::ranges::detail::LongT>::is_integer;
 } // namespace std
+
+namespace no_crash {
+struct A
+{
+  friend struct B;
+};
+
+struct B;
+
+template struct T {};
+
+T b;
+}



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

[PATCH] D131686: [clang-tidy] Add test to cert-dcl58-cpp (NFC).

[Balázs Kéri via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGab9ee471dfda: [clang-tidy] Add test to cert-dcl58-cpp (NFC). 
(authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131686

Files:
  clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp
@@ -268,3 +268,16 @@
 inline constexpr bool numeric_limits<::ranges::detail::LongT>::is_signed;
 inline constexpr bool numeric_limits<::ranges::detail::LongT>::is_integer;
 } // namespace std
+
+namespace no_crash {
+struct A
+{
+  friend struct B;
+};
+
+struct B;
+
+template struct T {};
+
+T b;
+}


Index: clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert/dcl58-cpp.cpp
@@ -268,3 +268,16 @@
 inline constexpr bool numeric_limits<::ranges::detail::LongT>::is_signed;
 inline constexpr bool numeric_limits<::ranges::detail::LongT>::is_integer;
 } // namespace std
+
+namespace no_crash {
+struct A
+{
+  friend struct B;
+};
+
+struct B;
+
+template struct T {};
+
+T b;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131992: [Support] compression proposal for a enum->spec->impl approach

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

@MaskRay I think this change is probably the best point of 
comparison/demonstration of this patch direction (taking some minor liberties 
with this patch to simplify things somewhat in ways that have already been 
discussed/seem quite reasonable - specifically having `getCompresisonSpec` 
return a reference and the enum having either no "unknown" value, or 
`getCompressionSpec` asserting on the unknown value):

  // removing the hardcoded zlib compression kind parameter in favor of what 
the code would look like in the near future once it's parameterized on a 
compression kind
  CompressionImplementation *Compressor = 
getCompressionSpec(CompressionAlgorithmKind)->Implementation;
  bool DoCompression = Compress && DoInstrProfNameCompression && Compressor;
  if (DoCompression) {
Compressor->compress(arrayRefFromStringRef(FilenamesStr),
CompressedStr,
Compressor->BestSizeLevel);
  }

I think a reasonable speculation about what the alternative would look like in 
the a non-OO design would be something like:

  bool DoCompression = Compress && DoInstrProfNameCompression && 
isAvailable(CompressionAlgorithmKind);
  if (DoCompression) {
compress(CompressionAlgorithmKind, arrayRefFromStringRef(FilenamesStr), 
CompressedStr, getBestSizeLevel(CompressionAlgorithmKind));

And having these three correlated calls (`isAvailable`, `compress`, and 
`getBestSizeLevel`) all made independently/have to have the same compression 
algorithm kind passed to them independently seems more error prone & redundant 
(not in an actual execution/runtime efficiency perspective - I don't think any 
of these different designs would have materially different runtime performance 
- but in terms of "duplicate work, and work that could diverge/become 
inconsistent" - essentially the duplicate work/repeated switch statements, one 
in each `compression::*` generic entry point seems like a code smell to me that 
points towards a design that doesn't have that duplication) rather than tying 
these calls together and making the lookup happen once and then using the 
features after that. Also exposing the compression algorithm along with the 
availability in one operation (not exposing compress/decompress/size APIs when 
the algorithm isn't available) seems to have similar benefits.

I agree it's somewhat overkill for two types of compression, but I don't think 
it's so burdensome as to be avoided either. I probably wouldn't've put quite 
this much consideration into the design if I were doing it myself amongst other 
things, but I appreciate that @ckissane has & I'm pretty happy with where this 
is now.

A few things still leftover that I think I've mentioned to @ckissane offline:

- getCompressionSpec returning by reference
- more incremental patches - implementing the generic API in terms of the old 
one (or keeping the old one as a wrapper around the generic one) with at most 
one or two uses of the API (if any) in the first commit - then subsequent API 
migrations in future commits, eventually removing the old API once the 
migration is complete
- having only the `getCompressionSpec(CompressionKind)` and dropping the 
version that takes uint8_t - callers should handle mapping from whatever 
domain-specific format/representation they have into the CompressionKind enum, 
there's no need for the raw version here
- Probably removing the 'unknown' CompressionKind - if a caller needs to handle 
having "maybe a compression kind, or none" they can wrap CompressionKind in 
Optional themselves and handle not looking up the algorithm if no compression 
kind is desired/requested




Comment at: llvm/lib/ProfileData/InstrProf.cpp:495
   return collectPGOFuncNameStrings(
-  NameStrs, compression::zlib::isAvailable() && doCompression, Result);
+  NameStrs, doCompression ? OptionalCompressionScheme : nullptr, Result);
 }

ckissane wrote:
> dblaikie wrote:
> > looks like this could be changed to pass the implementation, without the 
> > spec? (the caller doesn't need/use the spec)
> `(the caller doesn't need/use the spec)`...
> It definitely might in the future because it writes a header, that (in the 
> conceivably near future) (like one of my follow up patches) could contain the 
> uint 8 value of the compression type from the Spec, and 0 if uncompressed.
That should/can wait for the future change.

& I think at that point we could get into what the right design might be - and 
I was thinking maybe the right design might be for the 
CompressionImplementation to have a pointer back to its spec. But in general, 
lets defer that design decision for later - it's not too expensive to change 
what the parameter types are once the data is needed later.

(again, this might be even simpler addressed by not including this change at 
all up-front - keeping the old API and leaving these callers until you're 
patching the call

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

@Ericson2314 It seems that these changes break building on Windows using Visual 
Studio 2019.
There are 2 issues:

- CMake now creates a directory `$(Configuration)` in the root build directory.

  $(Configuration)\lib\cmake\llvm
  Location before the changes: lib\cmake\llvm

- The build fails with the errors:

  Error C1083 Cannot open include file: 'PPCGenRegisterInfo.inc': No such file 
or directory LLVMExegesisPowerPC 
llvm-project\llvm\lib\Target\PowerPC\MCTargetDesc\PPCMCTargetDesc.h
  Error C1083 Cannot open include file: 'MipsGenRegisterInfo.inc': No such file 
or directoryLLVMExegesisMips
llvm-project\llvm\lib\Target\Mips\MCTargetDesc\MipsMCTargetDesc.h
  Error C1083 Cannot open include file: 'X86GenRegisterInfo.inc': No such file 
or directory LLVMExegesisX86 
llvm-project\llvm\lib\Target\X86\MCTargetDesc\X86MCTargetDesc.h
  Error C1083 Cannot open include file: 'AArch64GenRegisterInfo.inc': No such 
file or directory LLVMExegesisAArch64 
llvm-project\llvm\lib\Target\AArch64\MCTargetDesc\AArch64MCTargetDesc.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

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

[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

[Dong-hee Na via Phabricator via cfe-commits]

corona10 created this revision.
Herald added subscribers: carlosgalvezp, jeroen.dobbelaere, xazax.hun.
Herald added a project: All.
corona10 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fix modernize-use-emplace to support alias cases


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132640

Files:
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp


Index: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -134,15 +134,21 @@
   // + match for emplace calls that should be replaced with insertion
   auto CallPushBack = cxxMemberCallExpr(
   hasDeclaration(functionDecl(hasName("push_back"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushBack);
+  on(anyOf(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushBack))),
+   hasType(hasCanonicalType(hasDeclaration(
+   namedDecl(hasAnyName(ContainersWithPushBack;
 
   auto CallPush = cxxMemberCallExpr(
   hasDeclaration(functionDecl(hasName("push"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPush);
+  on(anyOf(hasType(cxxRecordDecl(hasAnyName(ContainersWithPush))),
+   hasType(hasCanonicalType(hasDeclaration(
+   namedDecl(hasAnyName(ContainersWithPush;
 
   auto CallPushFront = cxxMemberCallExpr(
   hasDeclaration(functionDecl(hasName("push_front"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushFront);
+  on(anyOf(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushFront))),
+   hasType(hasCanonicalType(hasDeclaration(
+   namedDecl(hasAnyName(ContainersWithPushFront;
 
   auto CallEmplacy = cxxMemberCallExpr(
   hasDeclaration(


Index: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -134,15 +134,21 @@
   // + match for emplace calls that should be replaced with insertion
   auto CallPushBack = cxxMemberCallExpr(
   hasDeclaration(functionDecl(hasName("push_back"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushBack);
+  on(anyOf(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushBack))),
+   hasType(hasCanonicalType(hasDeclaration(
+   namedDecl(hasAnyName(ContainersWithPushBack;
 
   auto CallPush = cxxMemberCallExpr(
   hasDeclaration(functionDecl(hasName("push"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPush);
+  on(anyOf(hasType(cxxRecordDecl(hasAnyName(ContainersWithPush))),
+   hasType(hasCanonicalType(hasDeclaration(
+   namedDecl(hasAnyName(ContainersWithPush;
 
   auto CallPushFront = cxxMemberCallExpr(
   hasDeclaration(functionDecl(hasName("push_front"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushFront);
+  on(anyOf(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushFront))),
+   hasType(hasCanonicalType(hasDeclaration(
+   namedDecl(hasAnyName(ContainersWithPushFront;
 
   auto CallEmplacy = cxxMemberCallExpr(
   hasDeclaration(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 455516.
Sockke added a comment.

Rebased.


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

https://reviews.llvm.org/D127293

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
@@ -552,3 +552,30 @@
   int A;
   // CHECK-FIXES-NOT: int A{};
 };
+
+struct S1 {
+  S1() {}
+  union {
+int a = 0;
+int b;
+  };
+};
+
+struct S2 {
+  S2() : a{} {}
+  union {
+int a;
+int b;
+  };
+};
+
+struct S3 {
+  S3() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A [cppcoreguidelines-pro-type-member-init]
+  int A;
+  // CHECK-FIXES: int A{};
+  union {
+int B;
+int C = 0;
+  };
+};
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,11 @@
   ` check. Partial
   support for C++14 signal handler rules was added. Bug report generation was
   improved.
+
+- Fixed a false positive in :doc:`cppcoreguidelines-pro-type-member-init
+  ` when warnings
+  would be emitted for uninitialized members of an anonymous union despite
+  there being an initializer for one of the other members.
   
 - Improved `modernize-use-emplace `_ check.
 
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -61,6 +61,17 @@
   }
 }
 
+void removeFieldInitialized(const FieldDecl *M,
+SmallPtrSetImpl &FieldDecls) {
+  const RecordDecl *R = M->getParent();
+  if (R && R->isUnion()) {
+// Erase all members in a union if any member of it is initialized.
+for (const auto *F : R->fields())
+  FieldDecls.erase(F);
+  } else
+FieldDecls.erase(M);
+}
+
 void removeFieldsInitializedInBody(
 const Stmt &Stmt, ASTContext &Context,
 SmallPtrSetImpl &FieldDecls) {
@@ -70,7 +81,7 @@
 hasLHS(memberExpr(member(fieldDecl().bind("fieldDecl")),
 Stmt, Context);
   for (const auto &Match : Matches)
-FieldDecls.erase(Match.getNodeAs("fieldDecl"));
+removeFieldInitialized(Match.getNodeAs("fieldDecl"), FieldDecls);
 }
 
 StringRef getName(const FieldDecl *Field) { return Field->getName(); }
@@ -418,13 +429,20 @@
 
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
-  forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+  bool AnyMemberHasInitPerUnion = false;
+  forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
+ AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
 if (IgnoreArrays && F->getType()->isArrayType())
   return;
+if (F->hasInClassInitializer() && F->getParent()->isUnion()) {
+  AnyMemberHasInitPerUnion = true;
+  removeFieldInitialized(F, FieldsToInit);
+}
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&
-!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield())
+!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield() &&
+!AnyMemberHasInitPerUnion)
   FieldsToInit.insert(F);
   });
   if (FieldsToInit.empty())
@@ -437,7 +455,7 @@
   if (Init->isAnyMemberInitializer() && Init->isWritten()) {
 if (IsUnion)
   return; // We can only initialize one member of a union.
-FieldsToInit.erase(Init->getAnyMember());
+removeFieldInitialized(Init->getAnyMember(), FieldsToInit);
   }
 }
 removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
@@ -478,7 +496,7 @@
   // Collect all fields but only suggest a fix for the first member of unions,
   // as initializing more than one union member is an error.
   SmallPtrSet FieldsToFix;
-  bool AnyMemberHasInitPerUnion = false;
+  AnyMemberHasInitPerUnion = false;
   forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
  AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
 if (!FieldsToInit.count(F))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mail

[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

[Dong-hee Na via Phabricator via cfe-commits]

corona10 added a comment.

I am new to this project. I know that I need to write a test also likewise: 
https://github.com/llvm/llvm-project/blob/b8655f7ff286b9ebcd97cdd24b9c8eb5b89b9651/clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp#L872-L884
Please let me know how to generate CHECK-MESSAGES and CHECK-FIXES comments to 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132640

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

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added a comment.

In D131388#3748062 , @ChuanqiXu wrote:

> Replace `C++20 Modules` with `Standard C++ Modules` since @ruoso pointed out 
> that `Modules` is not specific to certain versions of C++ (for example, 
> Modules may get some big changes in C++23, C++26, ...).

The filename (and hence future doc URL) still contains the "20".


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

https://reviews.llvm.org/D131388

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

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne added a comment.

This is probably caused by using `CMAKE_CFG_INTDIR` (indirectly) in more 
places. Seems like it expands to `$(Configuration)` for Visual Studio.
Searching more about it, it’s only set at build time, but not at install time, 
which is a problem as well.
On top of all, `CMAKE_CFG_INTDIR` is deprecated and superseded by generator 
expression: https://cmake.org/cmake/help/latest/variable/CMAKE_CFG_INTDIR.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

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

[PATCH] D131501: [analyzer] Fix for incorrect handling of 0 length non-POD array construction

[Domján Dániel via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe3e9082b013b: [analyzer] Fix for incorrect handling of 0 
length non-POD array construction (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131501

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/array-init-loop.cpp
  clang/test/Analysis/dtor-array.cpp
  clang/test/Analysis/flexible-array-member.cpp
  clang/test/Analysis/zero-size-non-pod-array.cpp

Index: clang/test/Analysis/zero-size-non-pod-array.cpp
===
--- /dev/null
+++ clang/test/Analysis/zero-size-non-pod-array.cpp
@@ -0,0 +1,189 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct S{
+static int CtorInvocationCount;
+static int DtorInvocationCount;
+
+S(){CtorInvocationCount++;}
+~S(){DtorInvocationCount++;}
+};
+
+int S::CtorInvocationCount = 0;
+int S::DtorInvocationCount = 0;
+
+void zeroSizeArrayStack() {
+S::CtorInvocationCount = 0;
+
+S arr[0];
+
+clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+void zeroSizeMultidimensionalArrayStack() {
+S::CtorInvocationCount = 0;
+S::DtorInvocationCount = 0;
+
+{
+S arr[2][0];
+S arr2[0][2];
+
+S arr3[0][2][2];
+S arr4[2][2][0];
+S arr5[2][0][2];
+}
+
+clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+void zeroSizeArrayStackInLambda() {
+S::CtorInvocationCount = 0;
+S::DtorInvocationCount = 0;
+
+[]{
+S arr[0];
+}();
+
+clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+void zeroSizeArrayHeap() {
+S::CtorInvocationCount = 0;
+S::DtorInvocationCount = 0;
+
+auto *arr = new S[0];
+delete[] arr;
+
+clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+void zeroSizeMultidimensionalArrayHeap() {
+S::CtorInvocationCount = 0;
+S::DtorInvocationCount = 0;
+
+auto *arr = new S[2][0];
+delete[] arr;
+
+auto *arr2 = new S[0][2];
+delete[] arr2;
+
+auto *arr3 = new S[0][2][2];
+delete[] arr3;
+
+auto *arr4 = new S[2][2][0];
+delete[] arr4;
+
+auto *arr5 = new S[2][0][2];
+delete[] arr5;
+
+clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+#if __cplusplus >= 201703L
+
+void zeroSizeArrayBinding() {
+S::CtorInvocationCount = 0;
+
+S arr[0];
+
+// Note: This is an error in gcc but a warning in clang.
+// In MSVC the declaration of 'S arr[0]' is already an error
+// and it doesn't recognize this syntax as a structured binding.
+auto [] = arr; //expected-warning{{ISO C++17 does not allow a decomposition group to be empty}}
+
+clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+}
+
+#endif
+
+void zeroSizeArrayLambdaCapture() {
+S::CtorInvocationCount = 0;
+S::DtorInvocationCount = 0;
+
+S arr[0];
+
+auto l = [arr]{};
+[arr]{}();
+
+//FIXME: These should be TRUE. We should avoid calling the destructor 
+// of the temporary that is materialized as the lambda.
+clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}} expected-warning{{FALSE}}
+clang_analyzer_eval(S::DtorInvocationCount == 0); //expected-warning{{TRUE}} expected-warning{{FALSE}}
+}
+
+// FIXME: Report a warning if the standard is at least C++17.
+#if __cplusplus < 201703L
+void zeroSizeArrayLambdaCaptureUndefined1() {
+S arr[0];
+int n;
+
+auto l = [arr, n]{
+int x = n; //expected-warning{{Assigned value is garbage or undefined}}
+(void) x;
+};
+
+l();
+}
+#endif
+
+void zeroSizeArrayLambdaCaptureUndefined2() {
+S arr[0];
+int n;
+
+[arr, n]{
+int x = n; //expected-warning{{Assigned value is garbage or undefined}}
+(void) x;
+}();
+}
+
+struct Wrapper{
+S arr[0];
+};
+
+void zeroSizeArrayMember() {
+S::CtorInvocationCount = 0;
+S::DtorInvocationCount = 0;
+
+{
+Wrapper W;
+}
+
+clang_analyzer_eval(S::CtorInvocationCount == 0); //expected-warning{{TRUE}}
+clang_a

[clang] e3e9082 - [analyzer] Fix for incorrect handling of 0 length non-POD array construction

[via cfe-commits]

Author: isuckatcs
Date: 2022-08-25T12:42:02+02:00
New Revision: e3e9082b013ba948e3df6d4a2536df9d04656e76

URL: 
https://github.com/llvm/llvm-project/commit/e3e9082b013ba948e3df6d4a2536df9d04656e76
DIFF: 
https://github.com/llvm/llvm-project/commit/e3e9082b013ba948e3df6d4a2536df9d04656e76.diff

LOG: [analyzer] Fix for incorrect handling of 0 length non-POD array 
construction

Prior to this patch when the analyzer encountered a non-POD 0 length array,
it still invoked the constructor for 1 element, which lead to false positives.
This patch makes sure that we no longer construct any elements when we see a
0 length array.

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

Added: 
clang/test/Analysis/flexible-array-member.cpp
clang/test/Analysis/zero-size-non-pod-array.cpp

Modified: 
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/array-init-loop.cpp
clang/test/Analysis/dtor-array.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index eacacc4e1d611..ae878ecbcc34a 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -606,6 +606,27 @@ void ExprEngine::handleConstructor(const Expr *E,
 
 unsigned Idx = 0;
 if (CE->getType()->isArrayType() || AILE) {
+
+  auto isZeroSizeArray = [&] {
+uint64_t Size = 1;
+
+if (const auto *CAT = dyn_cast(CE->getType()))
+  Size = getContext().getConstantArrayElementCount(CAT);
+else if (AILE)
+  Size = getContext().getArrayInitLoopExprElementCount(AILE);
+
+return Size == 0;
+  };
+
+  // No element construction will happen in a 0 size array.
+  if (isZeroSizeArray()) {
+StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx);
+static SimpleProgramPointTag T{"ExprEngine",
+   "Skipping 0 size array construction"};
+Bldr.generateNode(CE, Pred, State, &T);
+return;
+  }
+
   Idx = getIndexOfElementToConstruct(State, CE, LCtx).value_or(0u);
   State = setIndexOfElementToConstruct(State, CE, LCtx, Idx + 1);
 }
@@ -1165,6 +1186,16 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, 
ExplodedNode *Pred,
 
   assert(InitExpr && "Capture missing initialization expression");
 
+  // Capturing a 0 length array is a no-op, so we ignore it to get a more
+  // accurate analysis. If it's not ignored, it would set the default
+  // binding of the lambda to 'Unknown', which can lead to falsely 
detecting
+  // 'Uninitialized' values as 'Unknown' and not reporting a warning.
+  const auto FTy = FieldForCapture->getType();
+  if (FTy->isConstantArrayType() &&
+  getContext().getConstantArrayElementCount(
+  getContext().getAsConstantArrayType(FTy)) == 0)
+continue;
+
   // With C++17 copy elision the InitExpr can be anything, so instead of
   // pattern matching all cases, we simple check if the current field is
   // under construction or not, regardless what it's InitExpr is.

diff  --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp 
b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index eca012b6015fa..4670ae0fb248e 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2594,6 +2594,12 @@ 
RegionStoreManager::tryBindSmallStruct(RegionBindingsConstRef B,
   return None;
 
 QualType Ty = FD->getType();
+
+// Zero length arrays are basically no-ops, so we also ignore them here.
+if (Ty->isConstantArrayType() &&
+Ctx.getConstantArrayElementCount(Ctx.getAsConstantArrayType(Ty)) == 0)
+  continue;
+
 if (!(Ty->isScalarType() || Ty->isReferenceType()))
   return None;
 

diff  --git a/clang/test/Analysis/array-init-loop.cpp 
b/clang/test/Analysis/array-init-loop.cpp
index ca7c832e8ab13..4ab4489fc882f 100644
--- a/clang/test/Analysis/array-init-loop.cpp
+++ b/clang/test/Analysis/array-init-loop.cpp
@@ -306,3 +306,27 @@ void structured_binding_multi() {
   clang_analyzer_eval(b[0].i == 3); // expected-warning{{TRUE}}
   clang_analyzer_eval(b[1].i == 4); // expected-warning{{TRUE}}
 }
+
+// This snippet used to crash
+namespace crash {
+
+struct S
+{
+  int x;
+  S() { x = 1; }
+};
+
+void no_crash() {
+  S arr[0];
+  int n = 1;
+
+  auto l = [arr, n] { return n; };
+
+  int x = l();
+  clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+
+  // FIXME: This should be 'Undefined'.
+  clang_analyzer_eval(arr[0].x); // expected-warning{{UNKNOWN}}
+}
+
+} // namespace crash

diff  --git a/clang/test/Analysis/dtor-array.cpp 
b/clang/test/Analysis/dtor-array.cpp
index da1c80e261948..ab0a939232173 100644
--- a/clang/test/Analysis/dtor-array.cpp
+++ b/clang/test/Analy

[clang-tools-extra] 2d8e91a - [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

[via cfe-commits]

Author: Sockke
Date: 2022-08-25T18:47:38+08:00
New Revision: 2d8e91a5d3597461b5ae2a40d9362abde67b5e1f

URL: 
https://github.com/llvm/llvm-project/commit/2d8e91a5d3597461b5ae2a40d9362abde67b5e1f
DIFF: 
https://github.com/llvm/llvm-project/commit/2d8e91a5d3597461b5ae2a40d9362abde67b5e1f.diff

LOG: [clang-tidy] Ignore other members in a union if any member of it is 
initialized in cppcoreguidelines-pro-type-member-init

If a union member is initialized, the other members of the union are still 
suggested to be initialized in this check.  This patch fixes this behavior.
Reference issue: https://github.com/llvm/llvm-project/issues/54748

Reviewed By: njames93

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
index 1540a451b46df..975b51cb3bfc5 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -61,6 +61,17 @@ void forEachFieldWithFilter(const RecordDecl &Record, const 
T &Fields,
   }
 }
 
+void removeFieldInitialized(const FieldDecl *M,
+SmallPtrSetImpl &FieldDecls) {
+  const RecordDecl *R = M->getParent();
+  if (R && R->isUnion()) {
+// Erase all members in a union if any member of it is initialized.
+for (const auto *F : R->fields())
+  FieldDecls.erase(F);
+  } else
+FieldDecls.erase(M);
+}
+
 void removeFieldsInitializedInBody(
 const Stmt &Stmt, ASTContext &Context,
 SmallPtrSetImpl &FieldDecls) {
@@ -70,7 +81,7 @@ void removeFieldsInitializedInBody(
 hasLHS(memberExpr(member(fieldDecl().bind("fieldDecl")),
 Stmt, Context);
   for (const auto &Match : Matches)
-FieldDecls.erase(Match.getNodeAs("fieldDecl"));
+removeFieldInitialized(Match.getNodeAs("fieldDecl"), 
FieldDecls);
 }
 
 StringRef getName(const FieldDecl *Field) { return Field->getName(); }
@@ -418,13 +429,20 @@ void 
ProTypeMemberInitCheck::checkMissingMemberInitializer(
 
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
-  forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+  bool AnyMemberHasInitPerUnion = false;
+  forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
+ AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
 if (IgnoreArrays && F->getType()->isArrayType())
   return;
+if (F->hasInClassInitializer() && F->getParent()->isUnion()) {
+  AnyMemberHasInitPerUnion = true;
+  removeFieldInitialized(F, FieldsToInit);
+}
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&
-!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield())
+!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield() &&
+!AnyMemberHasInitPerUnion)
   FieldsToInit.insert(F);
   });
   if (FieldsToInit.empty())
@@ -437,7 +455,7 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer(
   if (Init->isAnyMemberInitializer() && Init->isWritten()) {
 if (IsUnion)
   return; // We can only initialize one member of a union.
-FieldsToInit.erase(Init->getAnyMember());
+removeFieldInitialized(Init->getAnyMember(), FieldsToInit);
   }
 }
 removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
@@ -478,7 +496,7 @@ void ProTypeMemberInitCheck::checkMissingMemberInitializer(
   // Collect all fields but only suggest a fix for the first member of unions,
   // as initializing more than one union member is an error.
   SmallPtrSet FieldsToFix;
-  bool AnyMemberHasInitPerUnion = false;
+  AnyMemberHasInitPerUnion = false;
   forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
  AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
 if (!FieldsToInit.count(F))

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index fbe47316d23ae..1656debda1368 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,11 @@ Changes in existing checks
   ` check. Partial
   support for C++14 signal handler rules was added. Bug report generation was
   improved.
+
+- Fixed a false positive in :doc:`cppcoreguidelines-pro-type-member-init
+  ` when warnings
+  would be emitted for uninitialized members of an anonymou

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

[gehry via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2d8e91a5d359: [clang-tidy] Ignore other members in a union 
if any member of it is initialized… (authored by Sockke).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127293

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-member-init.cpp
@@ -552,3 +552,30 @@
   int A;
   // CHECK-FIXES-NOT: int A{};
 };
+
+struct S1 {
+  S1() {}
+  union {
+int a = 0;
+int b;
+  };
+};
+
+struct S2 {
+  S2() : a{} {}
+  union {
+int a;
+int b;
+  };
+};
+
+struct S3 {
+  S3() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A [cppcoreguidelines-pro-type-member-init]
+  int A;
+  // CHECK-FIXES: int A{};
+  union {
+int B;
+int C = 0;
+  };
+};
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,11 @@
   ` check. Partial
   support for C++14 signal handler rules was added. Bug report generation was
   improved.
+
+- Fixed a false positive in :doc:`cppcoreguidelines-pro-type-member-init
+  ` when warnings
+  would be emitted for uninitialized members of an anonymous union despite
+  there being an initializer for one of the other members.
   
 - Improved `modernize-use-emplace `_ check.
 
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -61,6 +61,17 @@
   }
 }
 
+void removeFieldInitialized(const FieldDecl *M,
+SmallPtrSetImpl &FieldDecls) {
+  const RecordDecl *R = M->getParent();
+  if (R && R->isUnion()) {
+// Erase all members in a union if any member of it is initialized.
+for (const auto *F : R->fields())
+  FieldDecls.erase(F);
+  } else
+FieldDecls.erase(M);
+}
+
 void removeFieldsInitializedInBody(
 const Stmt &Stmt, ASTContext &Context,
 SmallPtrSetImpl &FieldDecls) {
@@ -70,7 +81,7 @@
 hasLHS(memberExpr(member(fieldDecl().bind("fieldDecl")),
 Stmt, Context);
   for (const auto &Match : Matches)
-FieldDecls.erase(Match.getNodeAs("fieldDecl"));
+removeFieldInitialized(Match.getNodeAs("fieldDecl"), FieldDecls);
 }
 
 StringRef getName(const FieldDecl *Field) { return Field->getName(); }
@@ -418,13 +429,20 @@
 
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
-  forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+  bool AnyMemberHasInitPerUnion = false;
+  forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
+ AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
 if (IgnoreArrays && F->getType()->isArrayType())
   return;
+if (F->hasInClassInitializer() && F->getParent()->isUnion()) {
+  AnyMemberHasInitPerUnion = true;
+  removeFieldInitialized(F, FieldsToInit);
+}
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&
-!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield())
+!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield() &&
+!AnyMemberHasInitPerUnion)
   FieldsToInit.insert(F);
   });
   if (FieldsToInit.empty())
@@ -437,7 +455,7 @@
   if (Init->isAnyMemberInitializer() && Init->isWritten()) {
 if (IsUnion)
   return; // We can only initialize one member of a union.
-FieldsToInit.erase(Init->getAnyMember());
+removeFieldInitialized(Init->getAnyMember(), FieldsToInit);
   }
 }
 removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
@@ -478,7 +496,7 @@
   // Collect all fields but only suggest a fix for the first member of unions,
   // as initializing more than one union member is an error.
   SmallPtrSet FieldsToFix;
-  bool AnyMemberHasInitPerUnion = false;
+  AnyMemberHasInitPerUnion = f

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

This patch has been around for a long time, I have added the release notes. 
There are users waiting for this fix and I committed it myself first.
@LegalizeAdulthood I'm looking forward to your follow-up reply.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127293

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

[PATCH] D132643: [OpenMP] Extend lit test for parallel for simd construct

[Animesh Kumar via Phabricator via cfe-commits]

animeshk-amd created this revision.
animeshk-amd added reviewers: saiislam, JonChesterfield.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
animeshk-amd requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This construct is being tested for atomic operation based upon
the test 5.0/parallel_for_simd/test_parallel_for_simd_atomic.c
from the SOLLVE repo: https://github.com/SOLLVE/sollve_vv


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132643

Files:
  clang/test/OpenMP/parallel_for_simd_codegen.cpp


Index: clang/test/OpenMP/parallel_for_simd_codegen.cpp
===
--- clang/test/OpenMP/parallel_for_simd_codegen.cpp
+++ clang/test/OpenMP/parallel_for_simd_codegen.cpp
@@ -3,20 +3,20 @@
 // RUN: %clang_cc1 -no-opaque-pointers -fopenmp -fopenmp-version=45 -x c++ 
-triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions 
-debug-info-kind=limited -std=c++11 -include-pch %t -verify %s -emit-llvm -o - 
| FileCheck %s
 // RUN: %clang_cc1 -no-opaque-pointers -verify -triple x86_64-apple-darwin10 
-fopenmp -fopenmp-version=45 -fexceptions -fcxx-exceptions 
-debug-info-kind=line-tables-only -x c++ -emit-llvm %s -o - | FileCheck %s 
--check-prefix=TERM_DEBUG
 
-// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c++ -triple 
x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - | 
FileCheck %s --check-prefix=OMP50 --check-prefix=CHECK
-// RUN: %clang_cc1 -no-opaque-pointers -fopenmp -x c++ -std=c++11 -triple 
x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -no-opaque-pointers -fopenmp -x c++ -triple 
x86_64-unknown-unknown -fexceptions -fcxx-exceptions -debug-info-kind=limited 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
-// RUN: %clang_cc1 -no-opaque-pointers -verify -triple x86_64-apple-darwin10 
-fopenmp -fexceptions -fcxx-exceptions -debug-info-kind=line-tables-only -x c++ 
-emit-llvm %s -o - | FileCheck %s --check-prefix=TERM_DEBUG
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -DOMP5 -x c++ -triple 
x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - | 
FileCheck %s --check-prefix=OMP50 --check-prefix=CHECK
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp -DOMP5 -x c++ -std=c++11 
-triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp -DOMP5 -x c++ -triple 
x86_64-unknown-unknown -fexceptions -fcxx-exceptions -debug-info-kind=limited 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -verify -triple x86_64-apple-darwin10 
-fopenmp -DOMP5 -fexceptions -fcxx-exceptions -debug-info-kind=line-tables-only 
-x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix=TERM_DEBUG
 
 // RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp-simd 
-fopenmp-version=45 -x c++ -triple x86_64-unknown-unknown -emit-llvm %s 
-fexceptions -fcxx-exceptions -o - | FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -no-opaque-pointers -fopenmp-simd -fopenmp-version=45 -x 
c++ -std=c++11 -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions 
-emit-pch -o %t %s
 // RUN: %clang_cc1 -no-opaque-pointers -fopenmp-simd -fopenmp-version=45 -x 
c++ -triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions 
-debug-info-kind=limited -std=c++11 -include-pch %t -verify %s -emit-llvm -o - 
| FileCheck --check-prefix SIMD-ONLY0 %s
 // RUN: %clang_cc1 -no-opaque-pointers -verify -triple x86_64-apple-darwin10 
-fopenmp-simd -fopenmp-version=45 -fexceptions -fcxx-exceptions 
-debug-info-kind=line-tables-only -x c++ -emit-llvm %s -o - | FileCheck 
--check-prefix SIMD-ONLY0 %s
 
-// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp-simd  -x c++ -triple 
x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - | 
FileCheck --check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-simd  -x c++ -std=c++11 
-triple x86_64-unknown-unknown -fexceptions -fcxx-exceptions -emit-pch -o %t %s
-// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-simd  -x c++ -triple 
x86_64-unknown-unknown -fexceptions -fcxx-exceptions -debug-info-kind=limited 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck 
--check-prefix SIMD-ONLY0 %s
-// RUN: %clang_cc1 -no-opaque-pointers -verify -triple x86_64-apple-darwin10 
-fopenmp-simd  -fexceptions -fcxx-exceptions -debug-info-kind=line-tables-only 
-x c++ -emit-llvm %s -o - | FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp-simd -DOMP5 -x c++ 
-triple x86_64-unknown-unknown -emit-llvm %s -fexceptions -fcxx-exceptions -o - 
| FileCheck --check-prefix SIMD-ONLY0 %s
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp-simd -DOMP5 -x c++ -std=c++11 
-triple x86_64-unknown-unknown -f

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Was this reviewed by anyone on the original change? As far as I can tell, there 
was no agreement on the original change or here that reverting is the way to 
go. Was this discussed elsewhere?

(I don't have an opinion on which approach is better myself.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132486

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

[PATCH] D132486: SONAME introduce option CLANG_FORCE_MATCHING_LIBCLANG_SOVERSION

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D132486#3748546 , @thakis wrote:

> Was this reviewed by anyone on the original change? As far as I can tell, 
> there was no agreement on the original change or here that reverting is the 
> way to go. Was this discussed elsewhere?
>
> (I don't have an opinion on which approach is better myself.)

The bulk of the discussion was done in this thread: 
https://discourse.llvm.org/t/rationale-for-removing-versioned-libclang-middle-ground-to-keep-it-behind-option/64410

I think the original revert in https://reviews.llvm.org/D129160 was probably a 
little hasty. But since this was raised pretty late in the 15.x release process 
this is the compromise that was the least bad IMHO. I had some offline 
discussions with Tom and Hans about it as well and the conclusion is that we 
should decide a proper way forward but the end of 15.x release window was the 
wrong place to have that discussion. I hope we can sort this out before 16 is 
branched and released.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132486

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

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: cmake/Modules/GNUBinaryDirs.cmake:6
+if (NOT DEFINED CMAKE_BINARY_BINDIR)
+  set(CMAKE_BINARY_BINDIR "${CMAKE_BINARY_BINDIR}/bin")
+endif()

I find this name a bit confusing, maybe something like `CMAKE_BUILD_BINDIR` 
(and the same for _LIBDIR/_INCLUDEDIR would be less surprising? That way it's 
clear that we are referring to binaries/libraries/includes in the build output 
(and it mirrors CMAKE_INSTALL_INCLUDEDIR, etc.) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132608

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

[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

[Dong-hee Na via Phabricator via cfe-commits]

corona10 updated this revision to Diff 455536.

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

https://reviews.llvm.org/D132640

Files:
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp


Index: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -132,17 +132,20 @@
   // because this requires special treatment (it could cause performance
   // regression)
   // + match for emplace calls that should be replaced with insertion
-  auto CallPushBack = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push_back"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushBack);
-
-  auto CallPush = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPush);
-
-  auto CallPushFront = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push_front"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushFront);
+  auto CallPushBack =
+  cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push_back"))),
+on(hasType(hasCanonicalType(hasDeclaration(
+namedDecl(hasAnyName(ContainersWithPushBack)));
+
+  auto CallPush =
+  cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push"))),
+on(hasType(hasCanonicalType(hasDeclaration(
+namedDecl(hasAnyName(ContainersWithPush)));
+
+  auto CallPushFront =
+  cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push_front"))),
+on(hasType(hasCanonicalType(hasDeclaration(
+
namedDecl(hasAnyName(ContainersWithPushFront)));
 
   auto CallEmplacy = cxxMemberCallExpr(
   hasDeclaration(


Index: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -132,17 +132,20 @@
   // because this requires special treatment (it could cause performance
   // regression)
   // + match for emplace calls that should be replaced with insertion
-  auto CallPushBack = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push_back"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushBack);
-
-  auto CallPush = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPush);
-
-  auto CallPushFront = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push_front"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushFront);
+  auto CallPushBack =
+  cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push_back"))),
+on(hasType(hasCanonicalType(hasDeclaration(
+namedDecl(hasAnyName(ContainersWithPushBack)));
+
+  auto CallPush =
+  cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push"))),
+on(hasType(hasCanonicalType(hasDeclaration(
+namedDecl(hasAnyName(ContainersWithPush)));
+
+  auto CallPushFront =
+  cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push_front"))),
+on(hasType(hasCanonicalType(hasDeclaration(
+namedDecl(hasAnyName(ContainersWithPushFront)));
 
   auto CallEmplacy = cxxMemberCallExpr(
   hasDeclaration(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne added a comment.

I’m not sure if it’s the case for all places (as `CMAKE_CFG_INTDIR` is not 
defined at install time), but I think the `CMAKE_BINARY_LIBDIR` introduced here 
serves the same purpose as the already existing `LLVM_LIBRARY_DIR`.
Same for `CMAKE_BINARY_INCLUDEDIR`, which is `LLVM_INCLUDE_DIR` and 
`CMAKE_BINARY_BINDIR` which is `LLVM_TOOLS_BINARY_DIR`.

E.g. llvm/CMakeLists.txt uses

  set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )
  set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )

while clang uses

  set( CMAKE_LIBRARY_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
  set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY 
${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )

and this patch replaces the clang code with

  set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_LIBDIR} )
  set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_LIBDIR} )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132608

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

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

From looking at the output  in  the test cases, the additional diagnostics seem 
unnecessary to me in almost all cases...?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108211

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

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Frontend/SARIFDiagnostic.h:30-33
+  SARIFDiagnostic & operator= ( const SARIFDiagnostic && ) = delete;
+  SARIFDiagnostic ( SARIFDiagnostic && ) = delete;
+  SARIFDiagnostic &  operator= ( const SARIFDiagnostic & ) = delete;
+  SARIFDiagnostic ( const SARIFDiagnostic & ) = delete;

Formatting nits.



Comment at: clang/include/clang/Frontend/SARIFDiagnostic.h:45-47
+  void emitCodeContext(FullSourceLoc Loc, DiagnosticsEngine::Level Level,
+   SmallVectorImpl &Ranges,
+   ArrayRef Hints) override {}

Move this implementation to live with the rest and give it an assertion?



Comment at: clang/include/clang/Frontend/SARIFDiagnosticPrinter.h:72
+
+  bool OwnsOutputStream;
+};

This can go away, this never owns the output stream.



Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:42
+  // Build the SARIFDiagnostic utility.
+  assert(hasSarifWriter() && "Writer not set!");
+  SARIFDiag = std::make_unique(OS, LO, &*DiagOpts, &*Writer);

Should we also assert that we don't already have a valid `SARIFDiag` object (in 
case someone calls `BeginSourceFile()` twice)?



Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:53
+  OS.flush();
+}
+

Should we be invalidating that `SARIFDiag` object here so the printer cannot be 
used after the source file has ended?



Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:35-36
+SARIFDiagnosticPrinter::~SARIFDiagnosticPrinter() {
+  if (OwnsOutputStream)
+delete &OS;
+}

cjdb wrote:
> This makes me very uncomfortable. @aaron.ballman do we have a smart-pointer 
> that works like `variant>`?
No, but we shouldn't need it -- this object *never* owns the output stream, so 
I think we should remove the member variable and this code (we can default the 
dtor then).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

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

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D108211#3748595 , @tbaeder wrote:

> From looking at the output  in  the test cases, the additional diagnostics 
> seem unnecessary to me in almost all cases...?

+1; my observation is that the extra note repeats information because the new 
note says what the LHS and the RHS of the expression evaluates to. I could see 
this new note being more useful in a case like: `static_assert(sizeof(foo) + 
sizeof(bar) == sizeof(baz) / 12);` where there are multiple `sizeof` 
expressions involved and we could note what each constituent part evaluates to, 
but I'm not certain it's worth the extra complexity to go to those lengths (I'd 
want to see real world code where this would really help).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108211

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

[PATCH] D132640: [clang-tidy] Fix modernize-use-emplace to support alias cases

[Dong-hee Na via Phabricator via cfe-commits]

corona10 updated this revision to Diff 455544.
corona10 added a comment.

Add unittest


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

https://reviews.llvm.org/D132640

Files:
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
@@ -1061,6 +1061,44 @@
   // CHECK-FIXES: priority_queue.emplace(13);
 }
 
+void test_Alias() {
+  typedef std::list L;
+  using DQ = std::deque;
+  L l;
+  l.push_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back instead of 
push_back [modernize-use-emplace]
+  // CHECK-FIXES: l.emplace_back(3);
+
+  DQ dq;
+  dq.push_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of 
push_back [modernize-use-emplace]
+  // CHECK-FIXES: dq.emplace_back(3);
+
+  typedef std::stack STACK;
+  using PQ = std::priority_queue;
+  STACK stack;
+  stack.push(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use emplace instead of push 
[modernize-use-emplace]
+  // CHECK-FIXES: stack.emplace(3);
+
+  PQ pq;
+  pq.push(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace instead of push 
[modernize-use-emplace]
+  // CHECK-FIXES: pq.emplace(3);
+
+  typedef std::forward_list FL;
+  using DQ2 = std::deque;
+  FL fl;
+  fl.push_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_front instead of 
push_front [modernize-use-emplace]
+  // CHECK-FIXES: fl.emplace_front(3);
+
+  DQ2 dq2;
+  dq2.push_front(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use emplace_front instead of 
push_front [modernize-use-emplace]
+  // CHECK-FIXES: dq2.emplace_front(3);
+}
+
 struct Bar {
 public:
   Bar(){};
Index: clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
@@ -132,17 +132,20 @@
   // because this requires special treatment (it could cause performance
   // regression)
   // + match for emplace calls that should be replaced with insertion
-  auto CallPushBack = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push_back"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushBack);
-
-  auto CallPush = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPush);
-
-  auto CallPushFront = cxxMemberCallExpr(
-  hasDeclaration(functionDecl(hasName("push_front"))),
-  on(hasType(cxxRecordDecl(hasAnyName(ContainersWithPushFront);
+  auto CallPushBack =
+  cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push_back"))),
+on(hasType(hasCanonicalType(hasDeclaration(
+namedDecl(hasAnyName(ContainersWithPushBack)));
+
+  auto CallPush =
+  cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push"))),
+on(hasType(hasCanonicalType(hasDeclaration(
+namedDecl(hasAnyName(ContainersWithPush)));
+
+  auto CallPushFront =
+  cxxMemberCallExpr(hasDeclaration(functionDecl(hasName("push_front"))),
+on(hasType(hasCanonicalType(hasDeclaration(
+
namedDecl(hasAnyName(ContainersWithPushFront)));
 
   auto CallEmplacy = cxxMemberCallExpr(
   hasDeclaration(


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
@@ -1061,6 +1061,44 @@
   // CHECK-FIXES: priority_queue.emplace(13);
 }
 
+void test_Alias() {
+  typedef std::list L;
+  using DQ = std::deque;
+  L l;
+  l.push_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back instead of push_back [modernize-use-emplace]
+  // CHECK-FIXES: l.emplace_back(3);
+
+  DQ dq;
+  dq.push_back(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back instead of push_back [modernize-use-emplace]
+  // CHECK-FIXES: dq.emplace_back(3);
+
+  typedef std::stack STACK;
+  using PQ = std::priority_queue;
+  STACK stack;
+  stack.push(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use emplace instead of push [modernize-use-emplace]
+  // CHECK-FIXES: stack.emplace(3);
+
+  PQ pq;
+  pq.push(Foo(3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace instead of push [modernize-use-emplace]
+  // CHECK-FIXES: pq.emplace(3

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 455545.
carlosgalvezp added a comment.

Add a couple more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132461

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-do-while.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-do-while.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-do-while.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-do-while.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy -check-suffixes=DEFAULT   %s cppcoreguidelines-avoid-do-while %t
+// RUN: %check_clang_tidy -check-suffixes=ALLOW-WHILE-FALSE %s cppcoreguidelines-avoid-do-while %t -- -config='{CheckOptions: [{key: cppcoreguidelines-avoid-do-while.AllowWhileFalse, value: true}]}'
+
+#define FOO(x) \
+  do { \
+  } while (x != 0)
+
+#define BAR_0(x) \
+  do { \
+bar(x); \
+  } while (0)
+
+#define BAR_FALSE(x) \
+  do { \
+bar(x); \
+  } while (false)
+
+void bar(int);
+int baz();
+
+void foo()
+{
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: do-while loops shall not be used [cppcoreguidelines-avoid-do-while]
+do {
+
+} while(0);
+
+// CHECK-MESSAGES-ALLOW-WHILE-FALSE: :[[@LINE+2]]:5: warning: do-while loops shall not be used
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: do-while loops shall not be used
+do {
+
+} while(1);
+
+// CHECK-MESSAGES-ALLOW-WHILE-FALSE: :[[@LINE+2]]:5: warning: do-while loops shall not be used
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: do-while loops shall not be used
+do {
+
+} while(-1);
+
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: do-while loops shall not be used
+do {
+
+} while(false);
+
+// CHECK-MESSAGES-ALLOW-WHILE-FALSE: :[[@LINE+2]]:5: warning: do-while loops shall not be used
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: do-while loops shall not be used
+do {
+
+} while(true);
+
+// CHECK-MESSAGES-ALLOW-WHILE-FALSE: :[[@LINE+3]]:5: warning: do-while loops shall not be used
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+2]]:5: warning: do-while loops shall not be used
+int x1{0};
+do {
+  x1 = baz();
+} while (x1 > 0);
+
+// CHECK-MESSAGES-ALLOW-WHILE-FALSE: :[[@LINE+2]]:5: warning: do-while loops shall not be used
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: do-while loops shall not be used
+do {
+
+} while (x1 != 0);
+
+// CHECK-MESSAGES-ALLOW-WHILE-FALSE: :[[@LINE+3]]:5: warning: do-while loops shall not be used
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+2]]:5: warning: do-while loops shall not be used
+constexpr int x2{0};
+do {
+
+} while (x2);
+
+// CHECK-MESSAGES-ALLOW-WHILE-FALSE: :[[@LINE+3]]:5: warning: do-while loops shall not be used
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+2]]:5: warning: do-while loops shall not be used
+constexpr bool x3{false};
+do {
+
+} while (x3);
+
+// CHECK-MESSAGES-ALLOW-WHILE-FALSE: :[[@LINE+2]]:5: warning: do-while loops shall not be used
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: do-while loops shall not be used
+FOO(x1);
+
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: do-while loops shall not be used
+BAR_0(x1);
+
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: do-while loops shall not be used
+BAR_FALSE(x1);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -177,6 +177,7 @@
`concurrency-mt-unsafe `_,
`concurrency-thread-canceltype-asynchronous `_,
`cppcoreguidelines-avoid-const-or-ref-data-members `_,
+   `cppcoreguidelines-avoid-do-while `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-do-while.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-do-while.rst
@@ -0,0 +1,40 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-do-while
+
+cppcoreguidelines-avoid-do-while
+
+
+Warns when using ``do-while`` loops. They are less re

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

It also broke `llvm-exegesis` for 32 bits targets, see 
https://github.com/llvm/llvm-project/issues/57348
Reverting changes for the files in `tools/llvm-exegesis/` solves the 
compilation issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

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

[PATCH] D132111: [clang][Interp] Implement pointer (de)ref operations and DeclRefExprs

[Timm Bäder via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfdfc0dfa8ee3: [clang][Interp] Implement pointer (de)ref 
operators (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D132111?vs=454164&id=455546#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132111

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/test/AST/Interp/cxx20.cpp
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -31,3 +31,19 @@
 static_assert(!0, "");
 static_assert(-true, "");
 static_assert(-false, ""); //expected-error{{failed}} ref-error{{failed}}
+
+constexpr int m = 10;
+constexpr const int *p = &m;
+static_assert(p != nullptr, "");
+static_assert(*p == 10, "");
+
+constexpr const int* getIntPointer() {
+  return &m;
+}
+//static_assert(getIntPointer() == &m, ""); TODO
+//static_assert(*getIntPointer() == 10, ""); TODO
+
+constexpr int gimme(int k) {
+  return k;
+}
+// static_assert(gimme(5) == 5, ""); TODO
Index: clang/test/AST/Interp/cxx20.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/cxx20.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify=ref %s
+
+
+// expected-no-diagnostics
+// ref-no-diagnostics
+constexpr int getMinus5() {
+  int a = 10;
+  a = -5;
+  int *p = &a;
+  return *p;
+}
+//static_assert(getMinus5() == -5, "") TODO
+
+constexpr int assign() {
+  int m = 10;
+  int k = 12;
+
+  m = (k = 20);
+
+  return m;
+}
+//static_assert(assign() == 20, "");  TODO
+
+
+constexpr int pointerAssign() {
+  int m = 10;
+  int *p = &m;
+
+  *p = 12; // modifies m
+
+  return m;
+}
+//static_assert(pointerAssign() == 12, "");  TODO
+
+constexpr int pointerDeref() {
+  int m = 12;
+  int *p = &m;
+
+  return *p;
+}
+//static_assert(pointerDeref() == 12, ""); TODO
+
+constexpr int pointerAssign2() {
+  int m = 10;
+  int *p = &m;
+  int **pp = &p;
+
+  **pp = 12;
+
+  int v = **pp;
+
+  return v;
+}
+//static_assert(pointerAssign2() == 12, ""); TODO
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -72,6 +72,7 @@
   bool VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E);
   bool VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E);
   bool VisitUnaryOperator(const UnaryOperator *E);
+  bool VisitDeclRefExpr(const DeclRefExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -222,6 +222,10 @@
   return Discard(this->emitAdd(*T, BO));
 case BO_Mul:
   return Discard(this->emitMul(*T, BO));
+case BO_Assign:
+  if (!this->emitStore(*T, BO))
+return false;
+  return DiscardResult ? this->emitPopPtr(BO) : true;
 default:
   return this->bail(BO);
 }
@@ -609,8 +613,7 @@
 
 template 
 bool ByteCodeExprGen::VisitUnaryOperator(const UnaryOperator *E) {
-  if (!this->Visit(E->getSubExpr()))
-return false;
+  const Expr *SubExpr = E->getSubExpr();
 
   switch (E->getOpcode()) {
   case UO_PostInc: // x++
@@ -620,16 +623,32 @@
 return false;
 
   case UO_LNot: // !x
+if (!this->Visit(SubExpr))
+  return false;
 return this->emitInvBool(E);
   case UO_Minus: // -x
+if (!this->Visit(SubExpr))
+  return false;
 if (Optional T = classify(E->getType()))
   return this->emitNeg(*T, E);
 return false;
   case UO_Plus:  // +x
-return true; // noop
+return this->Visit(SubExpr); // noop
 
   case UO_AddrOf: // &x
+// We should already have a pointer when we get here.
+return this->Visit(SubExpr);
+
   case UO_Deref:  // *x
+return dereference(
+SubExpr, DerefKind::Read,
+[](PrimType) {
+  llvm_unreachable("Dereferencing requires a pointer");
+  return false;
+},
+[this, E](PrimType T) {
+  return DiscardResult ? this->emitPop(T, E) : true;
+});
   case UO_Not:// ~x
   case UO_Real:   // __real x
   case UO_Imag:   // __imag x
@@ -641,6 +660,23 @@
   return false;
 }
 
+template 
+bool ByteCodeExprGen::VisitDeclRefExpr(const DeclRefExpr *E) {
+  const auto *Decl = E->getDecl();
+
+  if (auto It = Locals.find(Decl); It != Locals.end()) {
+const unsigned Offset = It->second.Offset;
+return this->emitGetPtrLocal(Offset, E);
+ 

[clang] fdfc0df - [clang][Interp] Implement pointer (de)ref operators

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-08-25T14:20:32+02:00
New Revision: fdfc0dfa8ee3dc3d39741ac03a975917f28dcb46

URL: 
https://github.com/llvm/llvm-project/commit/fdfc0dfa8ee3dc3d39741ac03a975917f28dcb46
DIFF: 
https://github.com/llvm/llvm-project/commit/fdfc0dfa8ee3dc3d39741ac03a975917f28dcb46.diff

LOG: [clang][Interp] Implement pointer (de)ref operators

Implement pointer references, dereferences and assignments.

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

Added: 
clang/test/AST/Interp/cxx20.cpp

Modified: 
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/test/AST/Interp/literals.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index be2c48456b5b1..03f877197d160 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -222,6 +222,10 @@ bool ByteCodeExprGen::VisitBinaryOperator(const 
BinaryOperator *BO) {
   return Discard(this->emitAdd(*T, BO));
 case BO_Mul:
   return Discard(this->emitMul(*T, BO));
+case BO_Assign:
+  if (!this->emitStore(*T, BO))
+return false;
+  return DiscardResult ? this->emitPopPtr(BO) : true;
 default:
   return this->bail(BO);
 }
@@ -609,8 +613,7 @@ bool ByteCodeExprGen::VisitCXXNullPtrLiteralExpr(
 
 template 
 bool ByteCodeExprGen::VisitUnaryOperator(const UnaryOperator *E) {
-  if (!this->Visit(E->getSubExpr()))
-return false;
+  const Expr *SubExpr = E->getSubExpr();
 
   switch (E->getOpcode()) {
   case UO_PostInc: // x++
@@ -620,16 +623,32 @@ bool ByteCodeExprGen::VisitUnaryOperator(const 
UnaryOperator *E) {
 return false;
 
   case UO_LNot: // !x
+if (!this->Visit(SubExpr))
+  return false;
 return this->emitInvBool(E);
   case UO_Minus: // -x
+if (!this->Visit(SubExpr))
+  return false;
 if (Optional T = classify(E->getType()))
   return this->emitNeg(*T, E);
 return false;
   case UO_Plus:  // +x
-return true; // noop
+return this->Visit(SubExpr); // noop
 
   case UO_AddrOf: // &x
+// We should already have a pointer when we get here.
+return this->Visit(SubExpr);
+
   case UO_Deref:  // *x
+return dereference(
+SubExpr, DerefKind::Read,
+[](PrimType) {
+  llvm_unreachable("Dereferencing requires a pointer");
+  return false;
+},
+[this, E](PrimType T) {
+  return DiscardResult ? this->emitPop(T, E) : true;
+});
   case UO_Not:// ~x
   case UO_Real:   // __real x
   case UO_Imag:   // __imag x
@@ -641,6 +660,23 @@ bool ByteCodeExprGen::VisitUnaryOperator(const 
UnaryOperator *E) {
   return false;
 }
 
+template 
+bool ByteCodeExprGen::VisitDeclRefExpr(const DeclRefExpr *E) {
+  const auto *Decl = E->getDecl();
+
+  if (auto It = Locals.find(Decl); It != Locals.end()) {
+const unsigned Offset = It->second.Offset;
+return this->emitGetPtrLocal(Offset, E);
+  } else if (auto GlobalIndex = P.getGlobal(Decl)) {
+return this->emitGetPtrGlobal(*GlobalIndex, E);
+  } else if (const auto *PVD = dyn_cast(Decl)) {
+if (auto It = this->Params.find(PVD); It != this->Params.end())
+  return this->emitGetPtrParam(It->second, E);
+  }
+
+  return false;
+}
+
 template 
 void ByteCodeExprGen::emitCleanup() {
   for (VariableScope *C = VarScope; C; C = C->getParent())

diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.h 
b/clang/lib/AST/Interp/ByteCodeExprGen.h
index ae023aee9390d..f0459b55c8ef0 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -72,6 +72,7 @@ class ByteCodeExprGen : public 
ConstStmtVisitor, bool>,
   bool VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E);
   bool VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E);
   bool VisitUnaryOperator(const UnaryOperator *E);
+  bool VisitDeclRefExpr(const DeclRefExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;

diff  --git a/clang/test/AST/Interp/cxx20.cpp b/clang/test/AST/Interp/cxx20.cpp
new file mode 100644
index 0..30de029658338
--- /dev/null
+++ b/clang/test/AST/Interp/cxx20.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify 
%s
+// RUN: %clang_cc1 -std=c++20 -verify=ref %s
+
+
+// expected-no-diagnostics
+// ref-no-diagnostics
+constexpr int getMinus5() {
+  int a = 10;
+  a = -5;
+  int *p = &a;
+  return *p;
+}
+//static_assert(getMinus5() == -5, "") TODO
+
+constexpr int assign() {
+  int m = 10;
+  int k = 12;
+
+  m = (k = 20);
+
+  return m;
+}
+//static_assert(assign() == 20, "");  TODO
+
+
+constexpr int pointerAssign() {
+  int m = 10;
+  int *p = &m;
+
+  *p = 12; // modifies m
+
+  return m;
+}
+//static_assert(pointerAssign() == 12, "");  TODO
+
+constexpr int pointerDeref() {
+  int m = 12;
+  int *p = &m;
+
+  return *p

[clang] 9be8630 - Add a missing override keyword. NFC.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-08-25T14:50:28+02:00
New Revision: 9be8630ca9445b3370385bdbce9b7919f267e38a

URL: 
https://github.com/llvm/llvm-project/commit/9be8630ca9445b3370385bdbce9b7919f267e38a
DIFF: 
https://github.com/llvm/llvm-project/commit/9be8630ca9445b3370385bdbce9b7919f267e38a.diff

LOG: Add a missing override keyword. NFC.

clang/lib/Basic/Targets/X86.h:293:8: warning: 
'shouldEmitFloat16WithExcessPrecision' overrides a member function but is not 
marked 'override' [-Winconsistent-missing-override]
  bool shouldEmitFloat16WithExcessPrecision() const {
   ^
clang/include/clang/Basic/TargetInfo.h:915:16: note: overridden virtual 
function is here
  virtual bool shouldEmitFloat16WithExcessPrecision() const { return false; }
   ^

Added: 


Modified: 
clang/lib/Basic/Targets/X86.h

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index aef9f4a0676ec..7c1fe0d50fac0 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -290,7 +290,7 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public 
TargetInfo {
 return false;
   }
 
-  bool shouldEmitFloat16WithExcessPrecision() const {
+  bool shouldEmitFloat16WithExcessPrecision() const override {
 return HasFloat16 && !hasLegalHalfType();
   }
 



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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a subscriber: bkramer.
zahiraam added a comment.

Thanks @bkramer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113107

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

[PATCH] D131388: [docs] Add "Standard C++ Modules"

[Daniel Ruoso via Phabricator via cfe-commits]

ruoso added a comment.

I just noticed that the document was implying that Header Units were separate 
from the Standard C++ Modules, but they are an integral part of the language in 
the specification of modules. The contrast that we do is between "Named 
Modules" and "Header Units". I did some edits to help convey that.




Comment at: clang/docs/CPlusPlus20Modules.rst:24-26
+Although the term ``modules`` has a unique meaning in C++20 Language 
Specification,
+when people talk about standard C++ modules, they may refer to another C++20 
feature:
+header units, which are also covered in this document.





Comment at: clang/docs/CPlusPlus20Modules.rst:28-29
+
+standard C++ modules
+
+





Comment at: clang/docs/CPlusPlus20Modules.rst:285-287
+Another way to specify the dependent BMIs is to use ``-fmodule-file``. The 
main difference
+is that ``-fprebuilt-module-interface`` takes a directory, whereas 
``-fmodule-file`` requires a
+specific file.

Which one takes precedence?



Comment at: clang/docs/CPlusPlus20Modules.rst:585-586
+
+Similar to modules, we could use ``--precompile`` to produce the BMI.
+But we need to specify that the input file is a header by 
``-xc++-system-header`` or ``-xc++-user-header``.
+





Comment at: clang/docs/CPlusPlus20Modules.rst:728-729
+wrapping multiple headers together as a big module.
+However, these things are not part of C++20 Header units, and we want to avoid 
the impression that these
+additional semantics get interpreted as standard C++20 behavior.
+




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

https://reviews.llvm.org/D131388

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

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

The AllowWhileFlase option seems the wrong way to go about silencing do 
while(false) macros. Would it not make more sense to just ignore macros, or if 
you want more specificty don't warn on macros with a false condition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132461

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

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D132461#3748812 , @njames93 wrote:

> The AllowWhileFlase option seems the wrong way to go about silencing do 
> while(false) macros. Would it not make more sense to just ignore macros, or 
> if you want more specificty don't warn on macros with a false condition?

Strictly speaking, the C++ Core Guidelines do not make any such exception for 
macros. That's why I thought an option would allow both users who want to 
comply strictly, and users who want a bit more flexibility (without having to 
`NOLINT` their way out of it). Similar options exist for other C++ Core 
Guidelines checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132461

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

[PATCH] D132654: [clang-tidy] Fix false positive on `ArrayInitIndexExpr` inside `ProBoundsConstantArrayIndexCheck`

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs created this revision.
isuckatcs added reviewers: aaron.ballman, LegalizeAdulthood, njames93.
Herald added subscribers: carlosgalvezp, arphaman, kbarton, xazax.hun, nemanjai.
Herald added a project: All.
isuckatcs requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

There are 3 different cases when an `ArrayInitLoopExpr` is used to initialize 
an array.

- in the implicit copy/move constructor for a class with an array member
- when a lambda-expression captures an array by value
- when a decomposition declaration decomposes an array

The AST of such expression (usually) looks like this:

  |-ArrayInitLoopExpr 'int[3]'
  | | |-OpaqueValueExpr 'int[3]' lvalue
  | | | `-DeclRefExpr 'int[3]' lvalue Var 'arr' 'int[3]'
  | | `-ImplicitCastExpr 'int' 
  | |   `-ArraySubscriptExpr 'int' lvalue
  | | |-ImplicitCastExpr 'int *' 
  | | | `-OpaqueValueExpr 'int[3]' lvalue
  | | |   `-DeclRefExpr 'int[3]' lvalue Var 'arr' 'int[3]'
  | | `-ArrayInitIndexExpr <> 'unsigned long'

We basically always have an `ArraySubscriptExpr`, where the index is an 
`ArrayInitIndexExpr`.
`ArrayInitIndexExpr` is not a constant, so the checker mentioned in the title 
reports a warning.
This false positive warning only happens in case of decomposition declaration 
and lambda capture.

Some example without this patch:

  void foo() {
  int arr[3];
  
  auto [a, b, c] = arr;
  }
  ---
  warning: do not use array subscript when the index is not an integer constant 
expression [cppcoreguidelines-pro-bounds-constant-array-index]
  auto [a, b, c] = arr;
   ^



  void foo() {
  int arr[3];
  
  [arr]() {};
  }
  ---
  warning: do not use array subscript when the index is not an integer constant 
expression [cppcoreguidelines-pro-bounds-constant-array-index]
  [arr]() {};
   ^


https://reviews.llvm.org/D132654

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -1,4 +1,5 @@
 // RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index 
%t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index 
-extra-arg=-std=c++17 %t
 
 typedef __SIZE_TYPE__ size_t;
 
@@ -75,13 +76,34 @@
   s[i] = 3; // OK, custom operator
 }
 
+namespace ArrayInitIndexExpr {
 struct A {
   // The compiler-generated copy constructor uses an ArraySubscriptExpr. Don't 
warn.
   int x[3];
 };
 
-void use_A() {
+void implicitCopyMoveCtor() {
   // Force the compiler to generate a copy constructor.
   A a;
   A a2(a);
+
+  // Force the compiler to generate a move constructor.
+  A a3 = (A&&) a;
+}
+
+void lambdaCapture() {
+  int arr[3];
+
+  // Capturing an array by value uses an ArraySubscriptExpr. Don't warn. 
+  [arr](){};
+}
+
+#if __cplusplus >= 201703L
+void structuredBindings() {
+  int arr[3];
+
+  // Creating structured bindings by value uses an ArraySubscriptExpr. Don't 
warn.
+  auto [a,b,c] = arr;
 }
+#endif
+} // namespace ArrayInitIndexExpr
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -61,6 +61,12 @@
   const auto *Matched = Result.Nodes.getNodeAs("expr");
   const auto *IndexExpr = Result.Nodes.getNodeAs("index");
 
+  // This expression can only appear inside ArrayInitLoopExpr, which
+  // is always implicitly generated. ArrayInitIndexExpr is not a
+  // constant, but we shouldn't report a warning for it.
+  if (isa(IndexExpr))
+return;
+
   if (IndexExpr->isValueDependent())
 return; // We check in the specialization.
 


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -1,4 +1,5 @@
 // RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index -extra

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

Let's just revert this. I'll do it later today if no one beats me to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

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

[PATCH] D132617: [clang][deps] Minor ModuleDepCollector refactorings NFC

[Ben Langmuir via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc0a55121618b: [clang][deps] Minor ModuleDepCollector 
refactorings NFC (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132617

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -57,15 +57,7 @@
   // These are technically *inputs* to the compilation, but we populate them
   // here in order to make \c getModuleContextHash() independent of
   // \c lookupModuleOutput().
-  for (ModuleID MID : Deps.ClangModuleDeps) {
-auto PCMPath =
-Consumer.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
-if (EagerLoadModules)
-  CI.getFrontendOpts().ModuleFiles.push_back(PCMPath);
-else
-  CI.getHeaderSearchOpts().PrebuiltModuleFiles.insert(
-  {MID.ModuleName, PCMPath});
-  }
+  addModuleFiles(CI, Deps.ClangModuleDeps);
 
   CI.getFrontendOpts().OutputFile =
   Consumer.lookupModuleOutput(Deps.ID, ModuleOutputKind::ModuleFile);
@@ -125,24 +117,12 @@
   CI.getFrontendOpts().Inputs.emplace_back(Deps.ClangModuleMapFile,
ModuleMapInputKind);
   CI.getFrontendOpts().ModuleMapFiles = Deps.ModuleMapFileDeps;
+  addModuleMapFiles(CI, Deps.ClangModuleDeps);
 
   // Report the prebuilt modules this module uses.
   for (const auto &PrebuiltModule : Deps.PrebuiltModuleDeps)
 CI.getFrontendOpts().ModuleFiles.push_back(PrebuiltModule.PCMFile);
 
-  if (!EagerLoadModules) {
-ModuleMap &ModMap =
-ScanInstance.getPreprocessor().getHeaderSearchInfo().getModuleMap();
-for (ModuleID MID : Deps.ClangModuleDeps) {
-  const Module *M = ModMap.findModule(MID.ModuleName);
-  assert(M && "Modular dependency not found");
-  auto MDeps = ModularDeps.find(M);
-  assert(MDeps != ModularDeps.end() && "Inconsistent dependency info");
-  CI.getFrontendOpts().ModuleMapFiles.push_back(
-  MDeps->second->ClangModuleMapFile);
-}
-  }
-
   // Remove any macro definitions that are explicitly ignored.
   if (!CI.getHeaderSearchOpts().ModulesIgnoreMacros.empty()) {
 llvm::erase_if(
@@ -169,6 +149,31 @@
   return CI;
 }
 
+void ModuleDepCollector::addModuleMapFiles(
+CompilerInvocation &CI, ArrayRef ClangModuleDeps) const {
+  if (EagerLoadModules)
+return; // Only pcm is needed for eager load.
+
+  for (const ModuleID &MID : ClangModuleDeps) {
+ModuleDeps *MD = ModuleDepsByID.lookup(MID);
+assert(MD && "Inconsistent dependency info");
+CI.getFrontendOpts().ModuleMapFiles.push_back(MD->ClangModuleMapFile);
+  }
+}
+
+void ModuleDepCollector::addModuleFiles(
+CompilerInvocation &CI, ArrayRef ClangModuleDeps) const {
+  for (const ModuleID &MID : ClangModuleDeps) {
+std::string PCMPath =
+Consumer.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
+if (EagerLoadModules)
+  CI.getFrontendOpts().ModuleFiles.push_back(std::move(PCMPath));
+else
+  CI.getHeaderSearchOpts().PrebuiltModuleFiles.insert(
+  {MID.ModuleName, std::move(PCMPath)});
+  }
+}
+
 static std::string getModuleContextHash(const ModuleDeps &MD,
 const CompilerInvocation &CI,
 bool EagerLoadModules) {
@@ -210,6 +215,14 @@
   return toString(llvm::APInt(sizeof(Words) * 8, Words), 36, /*Signed=*/false);
 }
 
+void ModuleDepCollector::associateWithContextHash(const CompilerInvocation &CI,
+  ModuleDeps &Deps) {
+  Deps.ID.ContextHash = getModuleContextHash(Deps, CI, EagerLoadModules);
+  bool Inserted = ModuleDepsByID.insert({Deps.ID, &Deps}).second;
+  (void)Inserted;
+  assert(Inserted && "duplicate module mapping");
+}
+
 void ModuleDepCollectorPP::FileChanged(SourceLocation Loc,
FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -260,7 +273,8 @@
   const Module *TopLevelModule = Imported->getTopLevelModule();
 
   if (MDC.isPrebuiltModule(TopLevelModule))
-DirectPrebuiltModularDeps.insert(TopLevelModule);
+MDC.DirectPrebuiltModularDeps.insert(
+{TopLevelModule, PrebuiltModuleDep{TopLevelModule}});
   else
 DirectModularDeps.insert(TopLevelModule);
 }
@@ -297,8 +311,8 @@
   for (auto &&I : MDC.FileDeps)
 MDC.Consumer.handleFileDependency(I);
 
-  for (auto &&I : DirectPrebuiltModularDeps)
-MDC.Consumer.handlePrebuiltModuleDependency(PrebuiltMod

[clang] c0a5512 - [clang][deps] Minor ModuleDepCollector refactorings NFC

[Ben Langmuir via cfe-commits]

Author: Ben Langmuir
Date: 2022-08-25T06:51:06-07:00
New Revision: c0a55121618b2f3f5db613cfb0d104a9ae2b700e

URL: 
https://github.com/llvm/llvm-project/commit/c0a55121618b2f3f5db613cfb0d104a9ae2b700e
DIFF: 
https://github.com/llvm/llvm-project/commit/c0a55121618b2f3f5db613cfb0d104a9ae2b700e.diff

LOG: [clang][deps] Minor ModuleDepCollector refactorings NFC

* Factor module map and module file path functions out
* Use a secondary mapping to lookup module deps by ID instead of the
  preprocessor module map.
* Sink DirectPrebuiltModularDeps into MDC.

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

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h 
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 37eb4ef215b51..c0b7b2b57b583 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -61,12 +61,6 @@ struct ModuleID {
   }
 };
 
-struct ModuleIDHasher {
-  std::size_t operator()(const ModuleID &MID) const {
-return llvm::hash_combine(MID.ModuleName, MID.ContextHash);
-  }
-};
-
 /// An output from a module compilation, such as the path of the module file.
 enum class ModuleOutputKind {
   /// The module file (.pcm). Required.
@@ -153,8 +147,6 @@ class ModuleDepCollectorPP final : public PPCallbacks {
   ModuleDepCollector &MDC;
   /// Working set of direct modular dependencies.
   llvm::SetVector DirectModularDeps;
-  /// Working set of direct modular dependencies that have already been built.
-  llvm::SetVector DirectPrebuiltModularDeps;
 
   void handleImport(const Module *Imported);
 
@@ -211,6 +203,11 @@ class ModuleDepCollector final : public 
DependencyCollector {
   std::vector FileDeps;
   /// Direct and transitive modular dependencies of the main source file.
   llvm::MapVector> ModularDeps;
+  /// Secondary mapping for \c ModularDeps allowing lookup by ModuleID without
+  /// a preprocessor. Storage owned by \c ModularDeps.
+  llvm::DenseMap ModuleDepsByID;
+  /// Direct modular dependencies that have already been built.
+  llvm::MapVector DirectPrebuiltModularDeps;
   /// Options that control the dependency output generation.
   std::unique_ptr Opts;
   /// The original Clang invocation passed to dependency scanner.
@@ -235,12 +232,39 @@ class ModuleDepCollector final : public 
DependencyCollector {
   const ModuleDeps &Deps,
   llvm::function_ref Optimize) const;
 
+  /// Add module map files to the invocation, if needed.
+  void addModuleMapFiles(CompilerInvocation &CI,
+ ArrayRef ClangModuleDeps) const;
+  /// Add module files (pcm) to the invocation, if needed.
+  void addModuleFiles(CompilerInvocation &CI,
+  ArrayRef ClangModuleDeps) const;
+
   /// Add paths that require looking up outputs to the given dependencies.
   void addOutputPaths(CompilerInvocation &CI, ModuleDeps &Deps);
+
+  /// Compute the context hash for \p Deps, and create the mapping
+  /// \c ModuleDepsByID[Deps.ID] = &Deps.
+  void associateWithContextHash(const CompilerInvocation &CI, ModuleDeps 
&Deps);
 };
 
 } // end namespace dependencies
 } // end namespace tooling
 } // end namespace clang
 
+namespace llvm {
+template <> struct DenseMapInfo {
+  using ModuleID = clang::tooling::dependencies::ModuleID;
+  static inline ModuleID getEmptyKey() { return ModuleID{"", ""}; }
+  static inline ModuleID getTombstoneKey() {
+return ModuleID{"~", "~"}; // ~ is not a valid module name or context hash
+  }
+  static unsigned getHashValue(const ModuleID &ID) {
+return hash_combine(ID.ModuleName, ID.ContextHash);
+  }
+  static bool isEqual(const ModuleID &LHS, const ModuleID &RHS) {
+return LHS == RHS;
+  }
+};
+} // namespace llvm
+
 #endif // LLVM_CLANG_TOOLING_DEPENDENCYSCANNING_MODULEDEPCOLLECTOR_H

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index fe0de00f6244f..977ad23866ffc 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -57,15 +57,7 @@ void ModuleDepCollector::addOutputPaths(CompilerInvocation 
&CI,
   // These are technically *inputs* to the compilation, but we populate them
   // here in order to make \c getModuleContextHash() independent of
   // \c lookupModuleOutput().
-  for (ModuleID MID : Deps.ClangModuleDeps) {
-auto PCMPath =
-Consumer.lookupModuleOutput(MID, ModuleOutputKind::ModuleFile);
-if (EagerLoadModules)
-  CI.getFrontendOpts().ModuleFiles.push_back(PCMPath);
-else
-  CI.getHeaderSearchOpts().PrebuiltModuleF

[PATCH] D132659: PotentiallyEvaluatedContext in a ImmediateFunctionContext.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 created this revision.
Herald added a project: All.
usaxena95 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132659

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -908,3 +908,34 @@
   return testDefaultArgForParam() + testDefaultArgForParam((E)1);
 }
 }
+
+namespace GH51182 {
+// Nested consteval function.
+consteval int f(int v) {
+  return v;
+}
+
+template 
+consteval int g(T a) {
+  // An immediate function context.
+  int n = f(a);
+  return n;
+}
+static_assert(g(100) == 100);
+// --
+template 
+consteval T max(const T& a, const T& b) {
+return (a > b) ? a : b;
+}
+template 
+consteval T mid(const T& a, const T& b, const T& c) {
+T m = max(max(a, b), c);
+if (m == a)
+return max(b, c);
+if (m == b)
+return max(a, c);
+return max(a, b);
+}
+static_assert(max(1,2)==2);
+static_assert(mid(1,2,3)==2);
+} // namespace GH51182
\ No newline at end of file
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19676,8 +19676,8 @@
 
   if (auto *FD = dyn_cast(E->getDecl()))
 if (!isUnevaluatedContext() && !isConstantEvaluated() &&
-FD->isConsteval() && !RebuildingImmediateInvocation &&
-!FD->isDependentContext())
+!isImmediateFunctionContext() && FD->isConsteval() &&
+!RebuildingImmediateInvocation && !FD->isDependentContext())
   ExprEvalContexts.back().ReferenceToConsteval.insert(E);
   MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse,
  RefsMinusAssignments);
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14771,8 +14771,12 @@
   // Do not push if it is a lambda because one is already pushed when building
   // the lambda in ActOnStartOfLambdaDefinition().
   if (!isLambdaCallOperator(FD))
+// [expr.const#def:immediate_function_context]
+// An expression or conversion is in an immediate function context if it is
+// potentially evaluated and either: its innermost enclosing non-block 
scope
+// is a function parameter scope of an immediate function.
 PushExpressionEvaluationContext(
-FD->isConsteval() ? ExpressionEvaluationContext::ConstantEvaluated
+FD->isConsteval() ? 
ExpressionEvaluationContext::ImmediateFunctionContext
   : ExprEvalContexts.back().Context);
 
   // Check for defining attributes before the check for redefinition.
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1351,6 +1351,13 @@
 bool isImmediateFunctionContext() const {
   return Context == ExpressionEvaluationContext::ImmediateFunctionContext 
||
  (Context == ExpressionEvaluationContext::DiscardedStatement &&
+  InImmediateFunctionContext) ||
+ // [expr.const#def:immediate_function_context]
+ // An expression or conversion is in an immediate function
+ // context if it is potentially evaluated and either: its
+ // innermost enclosing non-block scope is a function parameter
+ // scope of an immediate function.
+ (Context == ExpressionEvaluationContext::PotentiallyEvaluated &&
   InImmediateFunctionContext);
 }
 


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -908,3 +908,34 @@
   return testDefaultArgForParam() + testDefaultArgForParam((E)1);
 }
 }
+
+namespace GH51182 {
+// Nested consteval function.
+consteval int f(int v) {
+  return v;
+}
+
+template 
+consteval int g(T a) {
+  // An immediate function context.
+  int n = f(a);
+  return n;
+}
+static_assert(g(100) == 100);
+// --
+template 
+consteval T max(const T& a, const T& b) {
+return (a > b) ? a : b;
+}
+template 
+consteval T mid(const T& a, const T& b, const T& c) {
+T m = max(max(a, b), c);
+if (m == a)
+return max(b, c);
+if (m == b)
+return max(a, c);
+return max(a, b);
+}
+static_assert(max(1,2)==2);
+static_assert(mid(1,2,3)==2);
+} // namespace GH51182
\ No newline at end of file
Index: clang/lib/Sema/SemaExpr.cpp
===

[PATCH] D132661: [clang] Make guard(nocf) attribute available only for Windows

[Alvin Wong via Phabricator via cfe-commits]

alvinhochun created this revision.
alvinhochun added reviewers: rnk, ajpaverd, mstorsjo, aaron.ballman.
Herald added a project: All.
alvinhochun requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Control Flow Guard is only supported on Windows target, therefore there
is no point to make it an accepted attribute for other targets.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132661

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td


Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetHasCFGuard : TargetSpec {
+  let CustomCode = [{ Target.getTriple().isOSWindows() }];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@
   let Documentation = [MSAllocatorDocs];
 }
 
-def CFGuard : InheritableAttr {
+def CFGuard : InheritableAttr, TargetSpecificAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In 
future
   // we might also want to support __declspec(guard(suppress)).
   let Spellings = [Declspec<"guard">, Clang<"guard">];
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -132,7 +132,8 @@
   ``[[clang::guard(nocf)]]``, which is equivalent to 
``__declspec(guard(nocf))``
   when using the MSVC environment. This is to support enabling Windows Control
   Flow Guard checks with the ability to disable them for specific functions 
when
-  using the MinGW environment.
+  using the MinGW environment. This attribute is only available for Windows
+  targets.
 
 Windows Support
 ---


Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -399,6 +399,9 @@
 def TargetX86 : TargetArch<["x86"]>;
 def TargetAnyX86 : TargetArch<["x86", "x86_64"]>;
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetHasCFGuard : TargetSpec {
+  let CustomCode = [{ Target.getTriple().isOSWindows() }];
+}
 def TargetHasDLLImportExport : TargetSpec {
   let CustomCode = [{ Target.getTriple().hasDLLImportExport() }];
 }
@@ -3494,7 +3497,7 @@
   let Documentation = [MSAllocatorDocs];
 }
 
-def CFGuard : InheritableAttr {
+def CFGuard : InheritableAttr, TargetSpecificAttr {
   // Currently only the __declspec(guard(nocf)) modifier is supported. In future
   // we might also want to support __declspec(guard(suppress)).
   let Spellings = [Declspec<"guard">, Clang<"guard">];
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -132,7 +132,8 @@
   ``[[clang::guard(nocf)]]``, which is equivalent to ``__declspec(guard(nocf))``
   when using the MSVC environment. This is to support enabling Windows Control
   Flow Guard checks with the ability to disable them for specific functions when
-  using the MinGW environment.
+  using the MinGW environment. This attribute is only available for Windows
+  targets.
 
 Windows Support
 ---
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] bd28bd5 - [clang-cl] /kernel should toggle bit 30 in @feat.00

[David Majnemer via cfe-commits]

Author: David Majnemer
Date: 2022-08-25T14:17:26Z
New Revision: bd28bd59a3693b6abc12231f2140e02760c0ada0

URL: 
https://github.com/llvm/llvm-project/commit/bd28bd59a3693b6abc12231f2140e02760c0ada0
DIFF: 
https://github.com/llvm/llvm-project/commit/bd28bd59a3693b6abc12231f2140e02760c0ada0.diff

LOG: [clang-cl] /kernel should toggle bit 30 in @feat.00

The linker is supposed to detect when an object with /kernel is linked
with another object which is not compiled with /kernel. The linker
detects this by checking bit 30 in @feat.00.

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/cfguardtable.c
llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
llvm/lib/Target/X86/X86AsmPrinter.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 6238a289978d..9df197056664 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -697,6 +697,10 @@ void CodeGenModule::Release() {
 // Function ID tables for EH Continuation Guard.
 getModule().addModuleFlag(llvm::Module::Warning, "ehcontguard", 1);
   }
+  if (Context.getLangOpts().Kernel) {
+// Note if we are compiling with /kernel.
+getModule().addModuleFlag(llvm::Module::Warning, "ms-kernel", 1);
+  }
   if (CodeGenOpts.OptimizationLevel > 0 && CodeGenOpts.StrictVTablePointers) {
 // We don't support LTO with 2 with 
diff erent StrictVTablePointers
 // FIXME: we could support it by stripping all the information introduced

diff  --git a/clang/test/CodeGen/cfguardtable.c 
b/clang/test/CodeGen/cfguardtable.c
index a92e1f6c610c..fe65d2a81c23 100644
--- a/clang/test/CodeGen/cfguardtable.c
+++ b/clang/test/CodeGen/cfguardtable.c
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -cfguard-no-checks -emit-llvm %s -o - | FileCheck %s 
-check-prefix=CFGUARDNOCHECKS
 // RUN: %clang_cc1 -cfguard -emit-llvm %s -o - | FileCheck %s 
-check-prefix=CFGUARD
 // RUN: %clang_cc1 -ehcontguard -emit-llvm %s -o - | FileCheck %s 
-check-prefix=EHCONTGUARD
+// RUN: %clang_cc1 -fms-kernel -emit-llvm %s -o - | FileCheck %s 
-check-prefix=MS-KERNEL
 
 void f(void) {}
 
@@ -8,3 +9,4 @@ void f(void) {}
 // CFGUARDNOCHECKS: !"cfguard", i32 1}
 // CFGUARD: !"cfguard", i32 2}
 // EHCONTGUARD: !"ehcontguard", i32 1}
+// MS-KERNEL: !"ms-kernel", i32 1}

diff  --git a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp 
b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
index 32d2f7945646..df2fce5621ad 100644
--- a/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
+++ b/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
@@ -219,6 +219,10 @@ void AArch64AsmPrinter::emitStartOfAsmFile(Module &M) {
   Feat00Flags |= 0x4000; // Object also has EHCont.
 }
 
+if (M.getModuleFlag("ms-kernel")) {
+  Feat00Flags |= 0x4000; // Object is compiled with /kernel.
+}
+
 OutStreamer->emitSymbolAttribute(S, MCSA_Global);
 OutStreamer->emitAssignment(
 S, MCConstantExpr::create(Feat00Flags, MMI->getContext()));

diff  --git a/llvm/lib/Target/X86/X86AsmPrinter.cpp 
b/llvm/lib/Target/X86/X86AsmPrinter.cpp
index 834b626dbb15..3e0f2a41ee74 100644
--- a/llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ b/llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -818,6 +818,10 @@ void X86AsmPrinter::emitStartOfAsmFile(Module &M) {
   Feat00Flags |= 0x4000; // Object also has EHCont.
 }
 
+if (M.getModuleFlag("ms-kernel")) {
+  Feat00Flags |= 0x4000; // Object is compiled with /kernel.
+}
+
 OutStreamer->emitSymbolAttribute(S, MCSA_Global);
 OutStreamer->emitAssignment(
 S, MCConstantExpr::create(Feat00Flags, MMI->getContext()));



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

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: dexonsmith, rjmccall.
aaron.ballman added a comment.

In D132568#3747598 , @nickdesaulniers 
wrote:

> In D132568#3746551 , @aaron.ballman 
> wrote:
>
>> Thank you for the patch, but it'd be really helpful to me as a reviewer if 
>> you and @nickdesaulniers could coordinate so there's only one patch trying 
>> to address #57102 instead of two competing patches (I'm happy to review 
>> whichever patch comes out). From a quick look over the test case changes, 
>> this patch looks like it's more in line with how I'd expect to resolve that 
>> issue compared to D132266 .
>
> The addition to clang/test/Sema/format-strings.c looks like it will resolve 
> Linus' complaints in https://github.com/llvm/llvm-project/issues/57102; I'm 
> happy to abandon D132266  in preference of 
> this.

Thanks, Nick!

I asked some questions about `wchar_t` behavior in the thread, but I sort of 
wonder if we should handle those concerns separately (if changes are needed) as 
it looks more like missing functionality than promotion-related behavior. We 
have other changes we'll need in this area anyway for C23 because of 
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2630.pdf, 
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2680.pdf, and 
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2763.pdf so there's no need 
to cram it all into one patch.




Comment at: clang/include/clang/AST/FormatString.h:263-264
 NoMatch = 0,
 /// The conversion specifier and the argument type are compatible. For
 /// instance, "%d" and _Bool.
 Match = 1,





Comment at: clang/lib/AST/FormatString.cpp:367
+case BuiltinType::Char_U:
+case BuiltinType::Bool:
+  return Match;

Isn't this a match promotion as well? e.g. `printf("%hhd", (_Bool)0);` where 
the `_Bool` is promoted to `int` but then cast back down to a char type (which 
seems rather unlikely to be a type confusion issue).



Comment at: clang/lib/AST/FormatString.cpp:378
+  case BuiltinType::Short:
+  case BuiltinType::UShort:
+return NoMatchPromotionTypeConfusion;

In C, `wchar_t` is an integer type and that underlying type might be `int` or 
it might be promotable to `int`. Should we handle it here as a type confused 
promotion match?



Comment at: clang/lib/AST/FormatString.cpp:401
+  if (const auto *BT = argTy->getAs()) {
+if (!Ptr) {
+  switch (BT->getKind()) {

It's a bit strange that we have two switches over the same `BT->getKind()` and 
the only difference is `!Ptr`; would it be easier to read if we combined the 
two switches into one and had logic in the individual cases for `Ptr` vs not 
`Ptr`?



Comment at: clang/lib/AST/FormatString.cpp:408
+if (T == C.SignedCharTy || T == C.UnsignedCharTy ||
+T == C.ShortTy || T == C.UnsignedShortTy)
+  return MatchPromotion;

`wchar_t`?



Comment at: clang/lib/AST/FormatString.cpp:413-414
+  case BuiltinType::UShort:
+if (T == C.SignedCharTy || T == C.UnsignedCharTy || T == C.IntTy ||
+T == C.UnsignedIntTy)
+  return NoMatchPromotionTypeConfusion;

I agree that `printf("%hhd", (short)0);` is potentially type confused, but why 
`printf("%d", (short)0);`?

FWIW, I think that `printf("%lc", (short)0));` is potentially type confused as 
well.



Comment at: clang/lib/AST/FormatString.cpp:422
+  case BuiltinType::Bool:
+// Don't warn printf("%hd", [char])
+// https://reviews.llvm.org/D66186

The comment is talking about passing a `char` (promoted to `int`) then printed 
as a `short` but the check is looking for the type to be printed as an `int`; 
that doesn't seem like a type confusion situation so much as a match due to 
promotion. Should the test below be looking for a short type instead of an int 
type?



Comment at: clang/lib/AST/FormatString.cpp:429
+  case BuiltinType::WChar_S:
+return NoMatchPromotionTypeConfusion;
+  }

Why is this automatically type confusion regardless of the type specified by 
the format string?



Comment at: clang/lib/Sema/SemaChecking.cpp:10114
   } else if (const CharacterLiteral *CL = dyn_cast(E)) {
 // Special case for 'a', which has type 'int' in C.
 // Note, however, that we do /not/ want to treat multibyte constants like

Do we need a similar special case for `L'a'` in C, which has type `wchar_t` 
(which is a typedef to an integer type)?



Comment at: clang/test/FixIt/format.m:40
 
-v

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki updated this revision to Diff 455579.
yusuke-kadowaki marked 9 inline comments as done.
yusuke-kadowaki added a comment.

- Remove my comments
- Update documentation
- Sort


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,107 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  //\n"
+   "\n"
+   "#include \"aa.h\" //\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"   //\n"
+   "\n"
+   "#include \"aa.h\"  //\n"
+   "\n"
+   "#include \"aaa.h\" //\n",   
+   Style);
+   
+  verifyFormat("#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n"
+   "#include \"aaa.h\"//\n"
+   "\n"
+   "#include \".h\"   //\n"
+   "#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  //\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" //\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   //\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  //\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" //\n",
+   Style);
+   
+  verifyFormat("#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n"
+   "#include \"aaa.h\"//\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   //\n"
+   "#include \"a.h\"  //\n"
+   "#include \"aa.h\" //\n",
+   Style);
+  
+  verifyFormat("#include \"a.h\"  //\n"
+   "\n"
+   "#include \"aa.h\" //\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\" // Do not align this because there are two lines without comments above\n",
+   Style);
+  
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+  
+  // FIXME: I think we need to change the implementations to pass tests below.
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line about b\n"
+   "   long b;",
+   Style));
+  
+  Style.ColumnLimit = 80;
+  EXPECT_EQ("int a; // line about a\n"
+"\n"
+"// line 1 about b\n"
+"// line 2 about b\n"
+"long b;",
+format("int a; // line about a\n"
+   "\n"
+   "   // line 1 about b\n"
+   "   // line 2 about b\n"
+   "   long b;",
+   Style));
+}
+
 TEST_F(FormatTestComments, AlignsBlockCommentDecorations) {
   EXPECT_EQ("/*\n"
 " */",
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20041,7 +20041,6 @@
 TEST_F(FormatTest, ParsesConfigurationBools) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
-  CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
@@ -20212,6 +20211,10 @@
   CHECK_ALIGN_CONSECUTIVE(AlignConsecutiveBitFields);
   CHECK_ALIGN_CONSECUTIVE(Align

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay requested changes to this revision.
MyDeveloperDay added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Format/Format.cpp:649
 IO.mapOptional("AlignOperands", Style.AlignOperands);
-IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
 IO.mapOptional("AllowAllArgumentsOnNextLine",

you can't remove an option, otherwise you'll break everyones .clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:20044
   Style.Language = FormatStyle::LK_Cpp;
-  CHECK_PARSE_BOOL(AlignTrailingComments);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);

don't remove


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D132503: Add cxx scope if needed for requires clause.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a subscriber: aaron.ballman.
ilya-biryukov added a comment.

Thanks for the fix. This looks ok to me, except that I am a bit suspicious of 
the fact that `DeclaratorScopeObj` is used somewhat rarely.
I suspect we might want a different guard class for this, e.g. something 
similar to `InitializerScopeRAII`.
But I do not enough of the details for the corresponding code to know what the 
implications of these choices are.

@aaron.ballman could you PTAL or suggest someone who can help review this 
change?




Comment at: clang/lib/Parse/ParseTemplate.cpp:294-299
+  if (DeclaratorInfo.getCXXScopeSpec().isValid()) {
+if (Actions.ShouldEnterDeclaratorScope(
+getCurScope(), DeclaratorInfo.getCXXScopeSpec())) {
+  DeclScopeObj.EnterDeclaratorScope();
+}
+  }

Merging ifs should produce simpler code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

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

[clang] 34fe6dd - Revert "[CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited"

[John Ericson via cfe-commits]

Author: John Ericson
Date: 2022-08-25T11:13:46-04:00
New Revision: 34fe6ddce11e4e0e31a96669ab5f200e5fb8a747

URL: 
https://github.com/llvm/llvm-project/commit/34fe6ddce11e4e0e31a96669ab5f200e5fb8a747
DIFF: 
https://github.com/llvm/llvm-project/commit/34fe6ddce11e4e0e31a96669ab5f200e5fb8a747.diff

LOG: Revert "[CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable 
are better-suited"

This reverts commit ad8c34bc3089d847a09bb740f7a58c96073e0959.

Added: 


Modified: 
bolt/lib/Target/AArch64/CMakeLists.txt
bolt/lib/Target/X86/CMakeLists.txt
bolt/unittests/Core/CMakeLists.txt
clang/cmake/modules/CMakeLists.txt
clang/lib/Tooling/CMakeLists.txt
flang/cmake/modules/CMakeLists.txt
lld/cmake/modules/CMakeLists.txt
lldb/cmake/modules/LLDBConfig.cmake
lldb/test/API/CMakeLists.txt
llvm/CMakeLists.txt
llvm/cmake/modules/AddLLVM.cmake
llvm/cmake/modules/CMakeLists.txt
llvm/tools/llvm-exegesis/lib/AArch64/CMakeLists.txt
llvm/tools/llvm-exegesis/lib/Mips/CMakeLists.txt
llvm/tools/llvm-exegesis/lib/PowerPC/CMakeLists.txt
llvm/tools/llvm-exegesis/lib/X86/CMakeLists.txt
llvm/tools/llvm-shlib/CMakeLists.txt
llvm/unittests/Target/ARM/CMakeLists.txt
llvm/unittests/Target/DirectX/CMakeLists.txt
llvm/unittests/tools/llvm-exegesis/AArch64/CMakeLists.txt
llvm/unittests/tools/llvm-exegesis/ARM/CMakeLists.txt
llvm/unittests/tools/llvm-exegesis/Mips/CMakeLists.txt
llvm/unittests/tools/llvm-exegesis/PowerPC/CMakeLists.txt
llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt
llvm/unittests/tools/llvm-mca/X86/CMakeLists.txt
mlir/cmake/modules/CMakeLists.txt
polly/cmake/CMakeLists.txt
polly/test/CMakeLists.txt

Removed: 




diff  --git a/bolt/lib/Target/AArch64/CMakeLists.txt 
b/bolt/lib/Target/AArch64/CMakeLists.txt
index 1e7ee71f2c30..96c70168196e 100644
--- a/bolt/lib/Target/AArch64/CMakeLists.txt
+++ b/bolt/lib/Target/AArch64/CMakeLists.txt
@@ -14,5 +14,5 @@ add_llvm_library(LLVMBOLTTargetAArch64
 
 include_directories(
   ${LLVM_MAIN_SRC_DIR}/lib/Target/AArch64
-  ${LLVM_LIBRARY_DIR}/Target/AArch64
+  ${LLVM_BINARY_DIR}/lib/Target/AArch64
   )

diff  --git a/bolt/lib/Target/X86/CMakeLists.txt 
b/bolt/lib/Target/X86/CMakeLists.txt
index a194148adf98..47344fe33111 100644
--- a/bolt/lib/Target/X86/CMakeLists.txt
+++ b/bolt/lib/Target/X86/CMakeLists.txt
@@ -17,5 +17,5 @@ add_llvm_library(LLVMBOLTTargetX86
 
 include_directories(
   ${LLVM_MAIN_SRC_DIR}/lib/Target/X86
-  ${LLVM_LIBRARY_DIR}/Target/X86
+  ${LLVM_BINARY_DIR}/lib/Target/X86
   )

diff  --git a/bolt/unittests/Core/CMakeLists.txt 
b/bolt/unittests/Core/CMakeLists.txt
index 9e9c1013b21f..0e78d0a2746f 100644
--- a/bolt/unittests/Core/CMakeLists.txt
+++ b/bolt/unittests/Core/CMakeLists.txt
@@ -20,7 +20,7 @@ target_link_libraries(CoreTests
 if ("AArch64" IN_LIST LLVM_TARGETS_TO_BUILD)
   include_directories(
 ${LLVM_MAIN_SRC_DIR}/lib/Target/AArch64
-${LLVM_LIBRARY_DIR}/Target/AArch64
+${LLVM_BINARY_DIR}/lib/Target/AArch64
   )
 
   target_compile_definitions(CoreTests PRIVATE AARCH64_AVAILABLE)
@@ -29,7 +29,7 @@ endif()
 if ("X86" IN_LIST LLVM_TARGETS_TO_BUILD)
   include_directories(
 ${LLVM_MAIN_SRC_DIR}/lib/Target/X86
-${LLVM_LIBRARY_DIR}/Target/X86
+${LLVM_BINARY_DIR}/lib/Target/X86
   )
 
   target_compile_definitions(CoreTests PRIVATE X86_AVAILABLE)

diff  --git a/clang/cmake/modules/CMakeLists.txt 
b/clang/cmake/modules/CMakeLists.txt
index 880d51f5aef7..6a7fa2fa27eb 100644
--- a/clang/cmake/modules/CMakeLists.txt
+++ b/clang/cmake/modules/CMakeLists.txt
@@ -15,7 +15,7 @@ set(clang_cmake_builddir 
"${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/cla
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to 
'${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
+set(llvm_cmake_builddir 
"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
 
 get_property(CLANG_EXPORTS GLOBAL PROPERTY CLANG_EXPORTS)
 export(TARGETS ${CLANG_EXPORTS} FILE 
${clang_cmake_builddir}/ClangTargets.cmake)

diff  --git a/clang/lib/Tooling/CMakeLists.txt 
b/clang/lib/Tooling/CMakeLists.txt
index e35e7f9be000..403d2dfb45e8 100644
--- a/clang/lib/Tooling/CMakeLists.txt
+++ b/clang/lib/Tooling/CMakeLists.txt
@@ -60,7 +60,7 @@ else()
   $
 # Skip this in debug mode because parsing AST.h is too slow
 --skip-processing=${skip_expensive_processing}
--I ${LLVM_LIBRARY_DIR}/clang/${CLANG_VERSION}/include
+-I ${LLVM_BINARY_DIR}/lib/clang/${CLANG_VERSION}/include
 -I ${CLANG_SOURCE_DIR}/include
 -I ${LLVM_BINARY_DIR}/tools/clang/include
 -I ${LLVM_BINARY_DIR}/include

diff  --git a/flang/cmake/modules/CMakeLists.txt 
b/flang/cmak

[PATCH] D132661: [clang] Make guard(nocf) attribute available only for Windows

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for this! Can you also add test coverage for the change?




Comment at: clang/include/clang/Basic/Attr.td:402
 def TargetWebAssembly : TargetArch<["wasm32", "wasm64"]>;
+def TargetHasCFGuard : TargetSpec {
+  let CustomCode = [{ Target.getTriple().isOSWindows() }];

How about we name this one `TargetWindows` instead of making it specific to 
just one attribute?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132661

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

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/test/FixIt/format.m:40
 
-void test_object_correction (id x) {  
+void test_object_correction (id x) {
   NSLog(@"%d", x); // expected-warning{{format specifies type 'int' but the 
argument has type 'id'}}

aaron.ballman wrote:
> It looks like some whitespace only changes snuck in.
> It looks like some whitespace only changes snuck in.

I'm sorry for this, do I need to discard these whitespace-only changes? Since 
this patch changes this file, is it my responsibility to keep other parts that 
don't belong to this patch unchanged, or am I correcting them by the way?



Comment at: clang/test/Sema/format-strings-scanf.c:289-290
+  // ill-formed floats
+  scanf("%hf", // expected-warning{{length modifier 'h' results in undefined 
behavior or no effect with 'f' conversion specifier}}
+  &sc); // Is this a clang bug ?
+

aaron.ballman wrote:
> Is what a clang bug?
> 
> Also, what is with the "or no effect" in that wording? "This could cause 
> major issues or nothing bad at all might happen" is a very odd way to word a 
> diagnostic. :-D (Maybe something to look into improving in a later patch.)
> Is what a clang bug?

Look at `sc` which is a `signed char`, but scanf need a pointer `float *`, I 
add this comment because I think there should be a warning like 
```
format specifies type 'float *' but the argument has type 'signed short *'
```

See printf tests below


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

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

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

[Carlos Alberto Enciso via Phabricator via cfe-commits]

CarlosAlbertoEnciso added a comment.

In D132316#3748845 , @Ericson2314 
wrote:

> Let's just revert this. I'll do it later today if no one beats me to it.

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

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

[PATCH] D127973: [analyzer] Eval construction of non POD type arrays.

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

@isuckatcs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127973

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

[PATCH] D127973: [analyzer] Eval construction of non POD type arrays.

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

This patch broke the `exploded-graph-rewriter`.
It prints this stack-trace, when I'm using the 
b032e3ff6121a969b2e90ad7bf493c2d5d7ac3a2 
. The 
issue is still present at main.

  Traceback (most recent call last):
File 
"/home/user/git/llvm-project/clang/utils/analyzer/exploded-graph-rewriter.py", 
line 1054, in 
  main()
File 
"/home/user/git/llvm-project/clang/utils/analyzer/exploded-graph-rewriter.py", 
line 1033, in main
  graph.add_raw_line(raw_line)
File 
"/home/user/git/llvm-project/clang/utils/analyzer/exploded-graph-rewriter.py", 
line 395, in add_raw_line
  self.nodes[node_id].construct(node_id, json_node)
File 
"/home/user/git/llvm-project/clang/utils/analyzer/exploded-graph-rewriter.py", 
line 322, in construct
  self.state = ProgramState(json_node['state_id'],
File 
"/home/user/git/llvm-project/clang/utils/analyzer/exploded-graph-rewriter.py", 
line 303, in __init__
  if json_ps['index_of_element'] is not None else None
  KeyError: 'index_of_element'

Unfortunately, I cannot share the egraph for reproduction, but I believe the 
trace should be enough for fixing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127973

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

[PATCH] D127973: [analyzer] Eval construction of non POD type arrays.

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs added a comment.

@steakhal

Can you send me a snippet please, which reproduces this issue? For me the 
egraph rewriter works fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127973

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

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/FixIt/format.m:40
 
-void test_object_correction (id x) {  
+void test_object_correction (id x) {
   NSLog(@"%d", x); // expected-warning{{format specifies type 'int' but the 
argument has type 'id'}}

inclyc wrote:
> aaron.ballman wrote:
> > It looks like some whitespace only changes snuck in.
> > It looks like some whitespace only changes snuck in.
> 
> I'm sorry for this, do I need to discard these whitespace-only changes? Since 
> this patch changes this file, is it my responsibility to keep other parts 
> that don't belong to this patch unchanged, or am I correcting them by the way?
> I'm sorry for this, do I need to discard these whitespace-only changes? Since 
> this patch changes this file, is it my responsibility to keep other parts 
> that don't belong to this patch unchanged, or am I correcting them by the way?

You should discard the whitespace only changes; you are welcome to land an NFC 
commit that fixes just the whitespace issues though!

We typically ask that changes unrelated to the patch summary should be 
separated out so that each patch is self-contained for just one thing. Not only 
is that easier to review, it also makes it less likely that we end up reverting 
good changes if we need to revert a patch.



Comment at: clang/test/Sema/format-strings-scanf.c:289-290
+  // ill-formed floats
+  scanf("%hf", // expected-warning{{length modifier 'h' results in undefined 
behavior or no effect with 'f' conversion specifier}}
+  &sc); // Is this a clang bug ?
+

inclyc wrote:
> aaron.ballman wrote:
> > Is what a clang bug?
> > 
> > Also, what is with the "or no effect" in that wording? "This could cause 
> > major issues or nothing bad at all might happen" is a very odd way to word 
> > a diagnostic. :-D (Maybe something to look into improving in a later patch.)
> > Is what a clang bug?
> 
> Look at `sc` which is a `signed char`, but scanf need a pointer `float *`, I 
> add this comment because I think there should be a warning like 
> ```
> format specifies type 'float *' but the argument has type 'signed short *'
> ```
> 
> See printf tests below
Oh I see what you're saying! There are two issues with the code: one is that 
the format specifier itself is UB and the other is that the argument passed to 
this confused format specifier does not agree.

To me, the priority is the invalid format specifier; unless the user corrects 
that, we're not really certain what they were aiming to scan in. And I think 
it's *probably* more likely that the user made a typo in the format specifier 
instead of trying to scan a half float (or whatever they thought they were 
scanning) into a `signed char`, so it seems defensible to silence the 
mismatched specifier diagnostic.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

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

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

So can this be merged now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129570

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

[PATCH] D132503: Add cxx scope if needed for requires clause.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: erichkeane, clang-language-wg.
aaron.ballman added a comment.

FWIW, I think this looks correct as well. I've added some reviewers just in 
case there's something I've missed regarding concepts.




Comment at: clang/lib/Parse/ParseTemplate.cpp:293-301
+  DeclaratorScopeObj DeclScopeObj(*this, DeclaratorInfo.getCXXScopeSpec());
+  if (DeclaratorInfo.getCXXScopeSpec().isValid()) {
+if (Actions.ShouldEnterDeclaratorScope(
+getCurScope(), DeclaratorInfo.getCXXScopeSpec())) {
+  DeclScopeObj.EnterDeclaratorScope();
+}
+  }

Can simplify even further with a new local.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

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

[PATCH] D132503: Add cxx scope if needed for requires clause.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Also, don't forget to add a release note since this is fixing an issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

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

[PATCH] D131683: Diagnosing the Future Keywords

[Muhammad Usman Shahid via Phabricator via cfe-commits]

Codesbyusman updated this revision to Diff 455607.
Codesbyusman added a comment.

updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131683

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/Lexer/keywords_test.c
  clang/test/Lexer/keywords_test.cpp
  clang/test/Parser/static_assert.c

Index: clang/test/Parser/static_assert.c
===
--- clang/test/Parser/static_assert.c
+++ clang/test/Parser/static_assert.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -verify=c2x %s
-// RUN: %clang_cc1 -fsyntax-only -std=c2x -DTEST_SPELLING -fms-compatibility -verify=c2x-ms %s
+// RUN: %clang_cc1 -fsyntax-only -std=c17 -DTEST_SPELLING -Wc2x-compat -verify=c17 %s
+// RUN: %clang_cc1 -fsyntax-only -std=c17 -DTEST_SPELLING -fms-compatibility -verify=c17-ms %s
 // RUN: %clang_cc1 -fsyntax-only -std=c2x -Wpre-c2x-compat -verify=c2x-compat %s
 // RUN: %clang_cc1 -fsyntax-only -std=c99 -verify=c99 %s
 // RUN: %clang_cc1 -fsyntax-only -std=c99 -pedantic -verify=c99-pedantic %s
@@ -15,11 +15,13 @@
 // Only test the C++ spelling in C mode in some of the tests, to reduce the
 // amount of diagnostics to have to check. This spelling is allowed in MS-
 // compatibility mode in C, but otherwise produces errors.
-static_assert(1, ""); // c2x-error {{expected parameter declarator}} \
-  // c2x-error {{expected ')'}} \
-  // c2x-note {{to match this '('}} \
-  // c2x-error {{a type specifier is required for all declarations}} \
-  // c2x-ms-warning {{use of 'static_assert' without inclusion of  is a Microsoft extension}}
+static_assert(1, ""); // c17-warning {{'static_assert' is a keyword in C2x}} \
+  // c17-error {{expected parameter declarator}} \
+  // c17-error {{expected ')'}} \
+  // c17-note {{to match this '('}} \
+  // c17-error {{type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int}} \
+  // c17-ms-warning {{use of 'static_assert' without inclusion of  is a Microsoft extension}} 
+ 
 #endif
 
 // We support _Static_assert as an extension in older C modes and in all C++
@@ -42,4 +44,7 @@
// cxx17-compat-warning {{'static_assert' with no message is incompatible with C++ standards before C++17}} \
// c99-pedantic-warning {{'_Static_assert' is a C11 extension}} \
// cxx17-pedantic-warning {{'_Static_assert' is a C11 extension}} \
-   // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}}
+   // cxx98-pedantic-warning {{'_Static_assert' is a C11 extension}} \
+   // c17-warning {{'_Static_assert' with no message is a C2x extension}} \
+   // c17-ms-warning {{'_Static_assert' with no message is a C2x extension}} 
+
Index: clang/test/Lexer/keywords_test.cpp
===
--- clang/test/Lexer/keywords_test.cpp
+++ clang/test/Lexer/keywords_test.cpp
@@ -15,6 +15,8 @@
 // RUN: %clang -std=c++03 -target i686-windows-msvc -DMS -fno-declspec -fsyntax-only %s
 // RUN: %clang -std=c++03 -target x86_64-scei-ps4 -fno-declspec -fsyntax-only %s
 
+// RUN: %clang_cc1 -std=c++98 -DFutureKeyword -fsyntax-only -Wc++11-compat -Wc++20-compat -verify=cxx98 %s
+
 #define IS_KEYWORD(NAME) _Static_assert(!__is_identifier(NAME), #NAME)
 #define NOT_KEYWORD(NAME) _Static_assert(__is_identifier(NAME), #NAME)
 #define IS_TYPE(NAME) void is_##NAME##_type() { int f(NAME); }
@@ -50,17 +52,24 @@
 CXX11_TYPE(char32_t);
 CXX11_KEYWORD(constexpr);
 CXX11_KEYWORD(noexcept);
+
 #ifndef MS
 CXX11_KEYWORD(static_assert);
 #else
 // MS compiler recognizes static_assert in all modes. So should we.
 IS_KEYWORD(static_assert);
 #endif
+
 CXX11_KEYWORD(thread_local);
 
 // Concepts keywords
 CXX20_KEYWORD(concept);
 CXX20_KEYWORD(requires);
+CXX20_KEYWORD(consteval);
+CXX20_KEYWORD(constinit);
+CXX20_KEYWORD(co_await);
+CXX20_KEYWORD(co_return);
+CXX20_KEYWORD(co_yield);
 
 // __declspec extension
 DECLSPEC_KEYWORD(__declspec);
@@ -70,3 +79,26 @@
 IS_TYPE(__char16_t);
 IS_KEYWORD(__char32_t);
 IS_TYPE(__char32_t);
+
+#ifdef FutureKeyword
+
+int nullptr; // cxx98-warning {{'nullptr' is a keyword in C++11}}
+int decltype;  // cxx98-warning {{'decltype' is a keyword in C++11}}
+int alignof;  // cxx98-warning {{'alignof' is a keyword in C++11}}
+int alignas;  // cxx98-warning {{'alignas' is a keyword in C++11}}
+int char16_t;  

[PATCH] D132503: Add cxx scope if needed for requires clause.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Like @ilya-biryukov , I don't really  know enough of the uses of 
'DeclaratorScopeObj ' to know if that is right, but other than Aaron's 
suggestions, I don't see anything to suggest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

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

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/lib/AST/FormatString.cpp:422
+  case BuiltinType::Bool:
+// Don't warn printf("%hd", [char])
+// https://reviews.llvm.org/D66186

aaron.ballman wrote:
> The comment is talking about passing a `char` (promoted to `int`) then 
> printed as a `short` but the check is looking for the type to be printed as 
> an `int`; that doesn't seem like a type confusion situation so much as a 
> match due to promotion. Should the test below be looking for a short type 
> instead of an int type?
> The comment is talking about passing a `char` (promoted to `int`) then 
> printed as a `short` but the check is looking for the type to be printed as 
> an `int`; that doesn't seem like a type confusion situation so much as a 
> match due to promotion. Should the test below be looking for a short type 
> instead of an int type?

Sorry for this (sometimes shit happens). There is a lot of redundant logic in 
this place, I will upload the corrected code immediately.



Comment at: clang/test/Sema/format-strings-scanf.c:289-290
+  // ill-formed floats
+  scanf("%hf", // expected-warning{{length modifier 'h' results in undefined 
behavior or no effect with 'f' conversion specifier}}
+  &sc); // Is this a clang bug ?
+

aaron.ballman wrote:
> inclyc wrote:
> > aaron.ballman wrote:
> > > Is what a clang bug?
> > > 
> > > Also, what is with the "or no effect" in that wording? "This could cause 
> > > major issues or nothing bad at all might happen" is a very odd way to 
> > > word a diagnostic. :-D (Maybe something to look into improving in a later 
> > > patch.)
> > > Is what a clang bug?
> > 
> > Look at `sc` which is a `signed char`, but scanf need a pointer `float *`, 
> > I add this comment because I think there should be a warning like 
> > ```
> > format specifies type 'float *' but the argument has type 'signed short *'
> > ```
> > 
> > See printf tests below
> Oh I see what you're saying! There are two issues with the code: one is that 
> the format specifier itself is UB and the other is that the argument passed 
> to this confused format specifier does not agree.
> 
> To me, the priority is the invalid format specifier; unless the user corrects 
> that, we're not really certain what they were aiming to scan in. And I think 
> it's *probably* more likely that the user made a typo in the format specifier 
> instead of trying to scan a half float (or whatever they thought they were 
> scanning) into a `signed char`, so it seems defensible to silence the 
> mismatched specifier diagnostic.
> 
> WDYT?
```  
printf("%hf", // expected-warning{{length modifier 'h' results in undefined 
behavior or no effect with 'f' conversion specifier}}
  sc); // expected-warning{{format specifies type 'double' but the argument has 
type 'signed char'}}
```

Then clang seems to have two different behaviors to `printf` and `scanf`, and 
for `printf` it will give the kind of diagnosis I said. I don't think this 
problem is particularly serious, because the key point is that using `%hf` UB. 
So maybe we can just keep clang as-is? Or we should consider implement the same 
warning in `scanf` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

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

[PATCH] D132592: [Clang] Implement function attribute nouwtable

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Do we have any evidence that users need this level of control or will 
understand how to properly use the attribute? The command line option makes 
sense to me because it's an all-or-nothing flag, but I'm not certain I 
understand the need for per-function control. Also, if a user gets this wrong 
(applies the attribute where they shouldn't), what is the failure mode (does it 
introduce any exploitable behavior)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132592

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

This patch has been open for weeks without maintainer input, despite repeated 
requests, and @abrahamcd finishes his internship this Friday. We would very 
much appreciate direction on whether or not D128372 
 is ready to be merged.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D132286: [clang][Interp] Implement function calls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

It looks like precommit CI found some relevant failures.




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:614
+
+if (Optional T = classify(E->getType())) {
+  // Put arguments on the stack.

tbaeder wrote:
> aaron.ballman wrote:
> > Should this be using `CallExpr::getCallReturnType()` instead? (with test 
> > cases demonstrating the difference between `getType()` and this call)
> Pretty sure yes, I just didn't know about it. Can the difference only be 
> demonstrated by using references?
I believe it's just references, but would have to go trace the logic through 
the compiler to be sure.



Comment at: clang/lib/AST/Interp/PrimType.h:85
   case Name: { using T = PrimConv::T; B; break; }
+
 #define TYPE_SWITCH(Expr, B)   
\

tbaeder wrote:
> aaron.ballman wrote:
> > Spurious whitespace change?
> It's on purpose, I was just trying to improve this part. But I can remove it.
It's the only change to the file, so it should be removed (you can make an NFC 
commit to fix all the whitespace issues though!)



Comment at: clang/test/AST/Interp/functions.cpp:1-2
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify 
%s
+// RUN: %clang_cc1 -std=c++14 -verify=ref %s
+

tbaeder wrote:
> aaron.ballman wrote:
> > I don't think we need to limit the testing to just c++14
> Isn't it now just testing whatever the default is when the test is run?
It is, but that's our preference for tests when possible because we don't want 
to lose testing in newer language modes by pegging the test to an older 
language mode (note, we're currently exploring changes to this so you can 
specify a range of standards: https://reviews.llvm.org/D131464).



Comment at: clang/test/AST/Interp/functions.cpp:8-9
+template constexpr T identity(T t) { return t; }
+static_assert(identity(true), "");
+static_assert(identity(true), "");
+static_assert(!identity(false), "");

tbaeder wrote:
> aaron.ballman wrote:
> > Why testing twice?
> The first call will compile the function in 
> `ByteCodeExprGen::VisitCallExpr()` and execute it, the second one checks that 
> visiting the same call again works. I can't test that the result is actually 
> cached and that the compilation only happens once.
Ah, that's good to know! That might be worth leaving a commend in the file so 
nobody thinks it's redundant.



Comment at: clang/test/AST/Interp/functions.cpp:48
+}
+static_assert(sub(10, 8, 2) == 0, "");

tbaeder wrote:
> aaron.ballman wrote:
> > It's a bit fuzzy where to draw the line on test coverage as this example is 
> > partially about function calls and partially about reference tracking. But 
> > I think we need more complex tests than what we've got so far.
> > ```
> > constexpr void func(int &ref) {
> >   ref = 12;
> > }
> > constexpr int do_it() {
> >   int mutate_me = 0;
> >   func(mutate_me);
> >   return mutate_me;
> > }
> > static_assert(do_it() == 12, "");
> > 
> > constexpr int &get_ref(int &i) {
> >   return i;
> > }
> > constexpr int do_it_again() {
> >   int i = 0;
> >   get_ref(i) = 12;
> >   return i;
> > }
> > static_assert(do_it_again() == 12, "");
> > ```
> > 
> > (Also, while working on this interpreter, please keep in mind that C is 
> > also looking to add support for constant expressions; C2x will have 
> > `constexpr` object definitions and we expect to widen that functionality 
> > for C2y. But this means you should keep in mind the crazy things that C can 
> > do too, like:
> > ```
> > int oh_god_this_hurts(struct { int a; } s) {
> >   return s.a;
> > }
> > ```
> > which could someday get a `constexpr` slapped in front of it.)
> > 
> > Speaking of C, remember that we also have C extensions, like using VM 
> > types, that should have test coverage:
> > ```
> > constexpr int func(int n, int array[static n], int m) {
> >   return array[m];
> > }
> > 
> > constexpr int do_it() {
> >   int array[] = { 0, 1, 2, 3, 4 };
> >   return func(5, array, 1);
> > }
> > 
> > static_assert(do_it() == 1, "");
> > ```
> TBH I think references can be implemented and tested in a separate, later 
> patch. Since I haven't tested them at all until now.
That's fine by me!


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

https://reviews.llvm.org/D132286

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

[clang] 22c477f - [HLSL] Initial codegen for SV_GroupIndex

[Chris Bieneman via cfe-commits]

Author: Chris Bieneman
Date: 2022-08-25T11:17:54-05:00
New Revision: 22c477f934c4d1fa3f7d782d32a3e151f581c686

URL: 
https://github.com/llvm/llvm-project/commit/22c477f934c4d1fa3f7d782d32a3e151f581c686
DIFF: 
https://github.com/llvm/llvm-project/commit/22c477f934c4d1fa3f7d782d32a3e151f581c686.diff

LOG: [HLSL] Initial codegen for SV_GroupIndex

Semantic parameters aren't passed as actual parameters, instead they are
populated from intrinsics which are generally lowered to reads from
dedicated hardware registers.

This change modifies clang CodeGen to emit the intrinsic calls and
populate the parameter's LValue with the result of the intrinsic call
for SV_GroupIndex.

The result of this is to make the actual passed argument ignored, which
will make it easy to clean up later in an IR pass.

Reviewed By: aaron.ballman

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

Added: 
clang/test/CodeGenHLSL/semantics/GroupIndex-codegen.hlsl

Modified: 
clang/lib/AST/Mangle.cpp
clang/lib/CodeGen/CGHLSLRuntime.cpp
clang/lib/CodeGen/CGHLSLRuntime.h
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenModule.cpp

Removed: 




diff  --git a/clang/lib/AST/Mangle.cpp b/clang/lib/AST/Mangle.cpp
index cb9a35cb0e7da..7ea569c63d9e4 100644
--- a/clang/lib/AST/Mangle.cpp
+++ b/clang/lib/AST/Mangle.cpp
@@ -133,10 +133,6 @@ bool MangleContext::shouldMangleDeclName(const NamedDecl 
*D) {
   if (isa(D))
 return true;
 
-  // HLSL shader entry function never need to be mangled.
-  if (getASTContext().getLangOpts().HLSL && D->hasAttr())
-return false;
-
   return shouldMangleCXXName(D);
 }
 

diff  --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp 
b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 5c3930869864c..fa7bab9121b7a 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -14,7 +14,9 @@
 
 #include "CGHLSLRuntime.h"
 #include "CodeGenModule.h"
+#include "clang/AST/Decl.h"
 #include "clang/Basic/TargetOptions.h"
+#include "llvm/IR/IntrinsicsDirectX.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 
@@ -87,11 +89,55 @@ void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, 
GlobalVariable *GV) {
 ConstantAsMetadata::get(B.getInt32(Counter))}));
 }
 
-void clang::CodeGen::CGHLSLRuntime::setHLSLFunctionAttributes(
-llvm::Function *F, const FunctionDecl *FD) {
-  if (HLSLShaderAttr *ShaderAttr = FD->getAttr()) {
-const StringRef ShaderAttrKindStr = "hlsl.shader";
-F->addFnAttr(ShaderAttrKindStr,
- ShaderAttr->ConvertShaderTypeToStr(ShaderAttr->getType()));
+void clang::CodeGen::CGHLSLRuntime::setHLSLEntryAttributes(
+const FunctionDecl *FD, llvm::Function *Fn) {
+  const auto *ShaderAttr = FD->getAttr();
+  assert(ShaderAttr && "All entry functions must have a HLSLShaderAttr");
+  const StringRef ShaderAttrKindStr = "hlsl.shader";
+  Fn->addFnAttr(ShaderAttrKindStr,
+ShaderAttr->ConvertShaderTypeToStr(ShaderAttr->getType()));
+}
+
+llvm::Value *CGHLSLRuntime::emitInputSemantic(IRBuilder<> &B,
+  const ParmVarDecl &D) {
+  assert(D.hasAttrs() && "Entry parameter missing annotation attribute!");
+  if (D.hasAttr()) {
+llvm::Function *DxGroupIndex =
+CGM.getIntrinsic(Intrinsic::dx_flattened_thread_id_in_group);
+return B.CreateCall(FunctionCallee(DxGroupIndex));
+  }
+  assert(false && "Unhandled parameter attribute");
+  return nullptr;
+}
+
+void CGHLSLRuntime::emitEntryFunction(const FunctionDecl *FD,
+  llvm::Function *Fn) {
+  llvm::Module &M = CGM.getModule();
+  llvm::LLVMContext &Ctx = M.getContext();
+  auto *EntryTy = llvm::FunctionType::get(llvm::Type::getVoidTy(Ctx), false);
+  Function *EntryFn =
+  Function::Create(EntryTy, Function::ExternalLinkage, FD->getName(), &M);
+
+  // Copy function attributes over, we have no argument or return attributes
+  // that can be valid on the real entry.
+  AttributeList NewAttrs = AttributeList::get(Ctx, 
AttributeList::FunctionIndex,
+  
Fn->getAttributes().getFnAttrs());
+  EntryFn->setAttributes(NewAttrs);
+  setHLSLEntryAttributes(FD, EntryFn);
+
+  // Set the called function as internal linkage.
+  Fn->setLinkage(GlobalValue::InternalLinkage);
+
+  BasicBlock *BB = BasicBlock::Create(Ctx, "entry", EntryFn);
+  IRBuilder<> B(BB);
+  llvm::SmallVector Args;
+  // FIXME: support struct parameters where semantics are on members.
+  for (const auto *Param : FD->parameters()) {
+Args.push_back(emitInputSemantic(B, *Param));
   }
+
+  CallInst *CI = B.CreateCall(FunctionCallee(Fn), Args);
+  (void)CI;
+  // FIXME: Handle codegen for return type semantics.
+  B.CreateRetVoid();
 }

diff  --git a/clang/lib/CodeGen/CGHLSLRuntime.h 
b/clang/lib/CodeGen/CGHLSLRuntime.h
index a7246f7f8919b..9bd698b325026 100644
-

[PATCH] D131203: [HLSL] Initial codegen for SV_GroupIndex

[Chris Bieneman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG22c477f934c4: [HLSL] Initial codegen for SV_GroupIndex 
(authored by beanz).

Changed prior to commit:
  https://reviews.llvm.org/D131203?vs=454677&id=455616#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131203

Files:
  clang/lib/AST/Mangle.cpp
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenHLSL/semantics/GroupIndex-codegen.hlsl

Index: clang/test/CodeGenHLSL/semantics/GroupIndex-codegen.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/semantics/GroupIndex-codegen.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - -hlsl-entry main %s
+
+[numthreads(1,1,1)]
+void main(unsigned GI : SV_GroupIndex) {
+  main(GI - 1);
+}
+
+// For HLSL entry functions, we are generating a C-export function that wraps
+// the C++-mangled entry function. The wrapper function can be used to populate
+// semantic parameters and provides the expected void(void) signature that
+// drivers expect for entry points.
+
+//CHECK: define void @main() #[[ENTRY_ATTR:#]]{
+//CHECK-NEXT: entry:
+//CHECK-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
+//CHECK-NEXT:   call void @"?main@@YAXI@Z"(i32 %0)
+//CHECK-NEXT:   ret void
+//CHECK-NEXT: }
+
+// Verify that the entry had the expected dx.shader attribute
+
+//CHECK: attributes #[[ENTRY_ATTR]] = { {{.*}}"dx.shader"="compute"{{.*}} }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1703,10 +1703,6 @@
  /*AttrOnCallSite=*/false, IsThunk);
   F->setAttributes(PAL);
   F->setCallingConv(static_cast(CallingConv));
-  if (getLangOpts().HLSL) {
-if (const FunctionDecl *FD = dyn_cast_or_null(GD.getDecl()))
-  getHLSLRuntime().setHLSLFunctionAttributes(F, FD);
-  }
 }
 
 static void removeImageAccessQualifier(std::string& TyName) {
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -16,6 +16,7 @@
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
 #include "CGDebugInfo.h"
+#include "CGHLSLRuntime.h"
 #include "CGOpenMPRuntime.h"
 #include "CodeGenModule.h"
 #include "CodeGenPGO.h"
@@ -1137,6 +1138,10 @@
   if (getLangOpts().OpenMP && CurCodeDecl)
 CGM.getOpenMPRuntime().emitFunctionProlog(*this, CurCodeDecl);
 
+  // Handle emitting HLSL entry functions.
+  if (D && D->hasAttr())
+CGM.getHLSLRuntime().emitEntryFunction(FD, Fn);
+
   EmitFunctionProlog(*CurFnInfo, CurFn, Args);
 
   if (isa_and_nonnull(D) &&
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -15,6 +15,8 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 #define LLVM_CLANG_LIB_CODEGEN_CGHLSLRUNTIME_H
 
+#include "llvm/IR/IRBuilder.h"
+
 #include "clang/Basic/HLSLRuntime.h"
 
 namespace llvm {
@@ -23,6 +25,7 @@
 } // namespace llvm
 namespace clang {
 class VarDecl;
+class ParmVarDecl;
 
 class FunctionDecl;
 
@@ -36,6 +39,8 @@
   uint32_t ResourceCounters[static_cast(
   hlsl::ResourceClass::NumClasses)] = {0};
 
+  llvm::Value *emitInputSemantic(llvm::IRBuilder<> &B, const ParmVarDecl &D);
+
 public:
   CGHLSLRuntime(CodeGenModule &CGM) : CGM(CGM) {}
   virtual ~CGHLSLRuntime() {}
@@ -44,7 +49,9 @@
 
   void finishCodeGen();
 
-  void setHLSLFunctionAttributes(llvm::Function *, const FunctionDecl *);
+  void setHLSLEntryAttributes(const FunctionDecl *FD, llvm::Function *Fn);
+
+  void emitEntryFunction(const FunctionDecl *FD, llvm::Function *Fn);
 };
 
 } // namespace CodeGen
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -14,7 +14,9 @@
 
 #include "CGHLSLRuntime.h"
 #include "CodeGenModule.h"
+#include "clang/AST/Decl.h"
 #include "clang/Basic/TargetOptions.h"
+#include "llvm/IR/IntrinsicsDirectX.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
 
@@ -87,11 +89,55 @@
 ConstantAsMetadata::get(B.getInt32(Counter))}));
 }
 
-void clang::CodeGen::CGHLSLRuntime::setHLSLFunctionAttributes(
-llvm::Function *F, const FunctionDecl *FD) {
-  if (HLSLShaderAttr *ShaderAttr = FD->getAttr()) {
-const StringRef ShaderAttrKindStr = "hlsl.shader";
-F->addFnAttr(ShaderAttrKindStr,
- ShaderAttr->ConvertShaderTypeToStr(ShaderAttr->getTy

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[YingChi Long via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/FormatString.cpp:367
+case BuiltinType::Char_U:
+case BuiltinType::Bool:
+  return Match;

aaron.ballman wrote:
> Isn't this a match promotion as well? e.g. `printf("%hhd", (_Bool)0);` where 
> the `_Bool` is promoted to `int` but then cast back down to a char type 
> (which seems rather unlikely to be a type confusion issue).
Hmm? `_Bool` just matches `AnyCharTy` perfectly. `MatchPromotion` means the 
types having **different** length and they can be considered as "partially 
matched" because of promotions). 

Isn't `_Bool`  and `AnyChar`  here just have the same length?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

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

[PATCH] D132670: [clang] Add cxx scope if needed for requires clause.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google created this revision.
Herald added a project: All.
luken-google requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixes issue #55216.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132670

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/Parser/cxx-concepts-requires-clause.cpp


Index: clang/test/Parser/cxx-concepts-requires-clause.cpp
===
--- clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -12,6 +12,7 @@
   struct AA;
   enum E : int;
   static int x;
+  static constexpr int z = 16;
 
   template  requires true
   void Mfoo();
@@ -24,6 +25,8 @@
 
   template  requires true
   using MQ = M;
+
+  constexpr int bazz() requires (z == 16);
 };
 
 template  requires (!0)
@@ -56,6 +59,9 @@
 template  requires true
 using Q = A;
 
+template requires (!0)
+constexpr int A::bazz() requires (z == 16) { return z; }
+
 struct C {
   template  requires true
   void Mfoo();
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -289,8 +289,14 @@
 
   LateParsedAttrList LateParsedAttrs(true);
   if (DeclaratorInfo.isFunctionDeclarator()) {
-if (Tok.is(tok::kw_requires))
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+  if (ScopeSpec.isValid() &&
+  Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+DeclScopeObj.EnterDeclaratorScope();
   ParseTrailingRequiresClause(DeclaratorInfo);
+}
 
 MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -167,8 +167,8 @@
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
 - Correctly defer dependent immediate function invocations until template 
instantiation.
   This fixes `GH55601 `_.
-
-
+- Class member variables are now in scope when parsing requires clauses. Fixes
+  `GH55216 `_.
 
 C++2b Feature Support
 ^


Index: clang/test/Parser/cxx-concepts-requires-clause.cpp
===
--- clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -12,6 +12,7 @@
   struct AA;
   enum E : int;
   static int x;
+  static constexpr int z = 16;
 
   template  requires true
   void Mfoo();
@@ -24,6 +25,8 @@
 
   template  requires true
   using MQ = M;
+
+  constexpr int bazz() requires (z == 16);
 };
 
 template  requires (!0)
@@ -56,6 +59,9 @@
 template  requires true
 using Q = A;
 
+template requires (!0)
+constexpr int A::bazz() requires (z == 16) { return z; }
+
 struct C {
   template  requires true
   void Mfoo();
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -289,8 +289,14 @@
 
   LateParsedAttrList LateParsedAttrs(true);
   if (DeclaratorInfo.isFunctionDeclarator()) {
-if (Tok.is(tok::kw_requires))
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+  if (ScopeSpec.isValid() &&
+  Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+DeclScopeObj.EnterDeclaratorScope();
   ParseTrailingRequiresClause(DeclaratorInfo);
+}
 
 MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -167,8 +167,8 @@
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
 - Correctly defer dependent immediate function invocations until template instantiation.
   This fixes `GH55601 `_.
-
-
+- Class member variables are now in scope when parsing requires clauses. Fixes
+  `GH55216 `_.
 
 C++2b Feature Support
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132503: Add cxx scope if needed for requires clause.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google updated this revision to Diff 455620.
luken-google added a comment.

Responding to feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/Parser/cxx-concepts-requires-clause.cpp


Index: clang/test/Parser/cxx-concepts-requires-clause.cpp
===
--- clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -12,6 +12,7 @@
   struct AA;
   enum E : int;
   static int x;
+  static constexpr int z = 16;
 
   template  requires true
   void Mfoo();
@@ -24,6 +25,8 @@
 
   template  requires true
   using MQ = M;
+
+  constexpr int bazz() requires (z == 16);
 };
 
 template  requires (!0)
@@ -56,6 +59,9 @@
 template  requires true
 using Q = A;
 
+template requires (!0)
+constexpr int A::bazz() requires (z == 16) { return z; }
+
 struct C {
   template  requires true
   void Mfoo();
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -289,8 +289,14 @@
 
   LateParsedAttrList LateParsedAttrs(true);
   if (DeclaratorInfo.isFunctionDeclarator()) {
-if (Tok.is(tok::kw_requires))
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+  if (ScopeSpec.isValid() &&
+  Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+DeclScopeObj.EnterDeclaratorScope();
   ParseTrailingRequiresClause(DeclaratorInfo);
+}
 
 MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -167,8 +167,8 @@
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
 - Correctly defer dependent immediate function invocations until template 
instantiation.
   This fixes `GH55601 `_.
-
-
+- Class member variables are now in scope when parsing requires clauses. Fixes
+  `GH55216 `_.
 
 C++2b Feature Support
 ^


Index: clang/test/Parser/cxx-concepts-requires-clause.cpp
===
--- clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -12,6 +12,7 @@
   struct AA;
   enum E : int;
   static int x;
+  static constexpr int z = 16;
 
   template  requires true
   void Mfoo();
@@ -24,6 +25,8 @@
 
   template  requires true
   using MQ = M;
+
+  constexpr int bazz() requires (z == 16);
 };
 
 template  requires (!0)
@@ -56,6 +59,9 @@
 template  requires true
 using Q = A;
 
+template requires (!0)
+constexpr int A::bazz() requires (z == 16) { return z; }
+
 struct C {
   template  requires true
   void Mfoo();
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -289,8 +289,14 @@
 
   LateParsedAttrList LateParsedAttrs(true);
   if (DeclaratorInfo.isFunctionDeclarator()) {
-if (Tok.is(tok::kw_requires))
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+  if (ScopeSpec.isValid() &&
+  Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+DeclScopeObj.EnterDeclaratorScope();
   ParseTrailingRequiresClause(DeclaratorInfo);
+}
 
 MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -167,8 +167,8 @@
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
 - Correctly defer dependent immediate function invocations until template instantiation.
   This fixes `GH55601 `_.
-
-
+- Class member variables are now in scope when parsing requires clauses. Fixes
+  `GH55216 `_.
 
 C++2b Feature Support
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/FormatString.cpp:422
+  case BuiltinType::Bool:
+// Don't warn printf("%hd", [char])
+// https://reviews.llvm.org/D66186

inclyc wrote:
> aaron.ballman wrote:
> > The comment is talking about passing a `char` (promoted to `int`) then 
> > printed as a `short` but the check is looking for the type to be printed as 
> > an `int`; that doesn't seem like a type confusion situation so much as a 
> > match due to promotion. Should the test below be looking for a short type 
> > instead of an int type?
> > The comment is talking about passing a `char` (promoted to `int`) then 
> > printed as a `short` but the check is looking for the type to be printed as 
> > an `int`; that doesn't seem like a type confusion situation so much as a 
> > match due to promotion. Should the test below be looking for a short type 
> > instead of an int type?
> 
> Sorry for this (sometimes shit happens). There is a lot of redundant logic in 
> this place, I will upload the corrected code immediately.
No worries!



Comment at: clang/test/Sema/format-strings-scanf.c:289-290
+  // ill-formed floats
+  scanf("%hf", // expected-warning{{length modifier 'h' results in undefined 
behavior or no effect with 'f' conversion specifier}}
+  &sc); // Is this a clang bug ?
+

inclyc wrote:
> aaron.ballman wrote:
> > inclyc wrote:
> > > aaron.ballman wrote:
> > > > Is what a clang bug?
> > > > 
> > > > Also, what is with the "or no effect" in that wording? "This could 
> > > > cause major issues or nothing bad at all might happen" is a very odd 
> > > > way to word a diagnostic. :-D (Maybe something to look into improving 
> > > > in a later patch.)
> > > > Is what a clang bug?
> > > 
> > > Look at `sc` which is a `signed char`, but scanf need a pointer `float 
> > > *`, I add this comment because I think there should be a warning like 
> > > ```
> > > format specifies type 'float *' but the argument has type 'signed short *'
> > > ```
> > > 
> > > See printf tests below
> > Oh I see what you're saying! There are two issues with the code: one is 
> > that the format specifier itself is UB and the other is that the argument 
> > passed to this confused format specifier does not agree.
> > 
> > To me, the priority is the invalid format specifier; unless the user 
> > corrects that, we're not really certain what they were aiming to scan in. 
> > And I think it's *probably* more likely that the user made a typo in the 
> > format specifier instead of trying to scan a half float (or whatever they 
> > thought they were scanning) into a `signed char`, so it seems defensible to 
> > silence the mismatched specifier diagnostic.
> > 
> > WDYT?
> ```  
> printf("%hf", // expected-warning{{length modifier 'h' results in undefined 
> behavior or no effect with 'f' conversion specifier}}
>   sc); // expected-warning{{format specifies type 'double' but the argument 
> has type 'signed char'}}
> ```
> 
> Then clang seems to have two different behaviors to `printf` and `scanf`, and 
> for `printf` it will give the kind of diagnosis I said. I don't think this 
> problem is particularly serious, because the key point is that using `%hf` 
> UB. So maybe we can just keep clang as-is? Or we should consider implement 
> the same warning in `scanf` ?
> So maybe we can just keep clang as-is?

I think let's keep clang as-is for the moment so we can keep the concerns 
separate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

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

[PATCH] D132503: Add cxx scope if needed for requires clause.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google marked 2 inline comments as done.
luken-google added a comment.

Thanks for the feedback and the code review. I've uploaded a new revision with 
the requested changes, PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

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

[PATCH] D132670: [clang] Add cxx scope if needed for requires clause.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google abandoned this revision.
luken-google added a comment.

Uploaded by mistake, kindly ignore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132670

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

[PATCH] D132503: Add cxx scope if needed for requires clause.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Parse/ParseTemplate.cpp:293
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);

Not sure about this type, should this be a reference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

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

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/FormatString.cpp:367
+case BuiltinType::Char_U:
+case BuiltinType::Bool:
+  return Match;

inclyc wrote:
> aaron.ballman wrote:
> > Isn't this a match promotion as well? e.g. `printf("%hhd", (_Bool)0);` 
> > where the `_Bool` is promoted to `int` but then cast back down to a char 
> > type (which seems rather unlikely to be a type confusion issue).
> Hmm? `_Bool` just matches `AnyCharTy` perfectly. `MatchPromotion` means the 
> types having **different** length and they can be considered as "partially 
> matched" because of promotions). 
> 
> Isn't `_Bool`  and `AnyChar`  here just have the same length?
> Hmm? _Bool just matches AnyCharTy perfectly. MatchPromotion means the types 
> having different length and they can be considered as "partially matched" 
> because of promotions).
>
> Isn't _Bool and AnyChar here just have the same length?

`_Bool` is not a character type, so it doesn't match `AnyCharTy` perfectly. The 
size can be the same but they're still different types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

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

[PATCH] D132503: [clang] Add cxx scope if needed for requires clause.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google updated this revision to Diff 455622.
luken-google retitled this revision from "Add cxx scope if needed for requires 
clause." to "[clang] Add cxx scope if needed for requires clause.".
luken-google added a comment.

Make ScopeSpec a reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/Parser/cxx-concepts-requires-clause.cpp


Index: clang/test/Parser/cxx-concepts-requires-clause.cpp
===
--- clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -12,6 +12,7 @@
   struct AA;
   enum E : int;
   static int x;
+  static constexpr int z = 16;
 
   template  requires true
   void Mfoo();
@@ -24,6 +25,8 @@
 
   template  requires true
   using MQ = M;
+
+  constexpr int bazz() requires (z == 16);
 };
 
 template  requires (!0)
@@ -56,6 +59,9 @@
 template  requires true
 using Q = A;
 
+template requires (!0)
+constexpr int A::bazz() requires (z == 16) { return z; }
+
 struct C {
   template  requires true
   void Mfoo();
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -289,8 +289,14 @@
 
   LateParsedAttrList LateParsedAttrs(true);
   if (DeclaratorInfo.isFunctionDeclarator()) {
-if (Tok.is(tok::kw_requires))
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec &ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+  if (ScopeSpec.isValid() &&
+  Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+DeclScopeObj.EnterDeclaratorScope();
   ParseTrailingRequiresClause(DeclaratorInfo);
+}
 
 MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -167,8 +167,8 @@
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
 - Correctly defer dependent immediate function invocations until template 
instantiation.
   This fixes `GH55601 `_.
-
-
+- Class member variables are now in scope when parsing requires clauses. Fixes
+  `GH55216 `_.
 
 C++2b Feature Support
 ^


Index: clang/test/Parser/cxx-concepts-requires-clause.cpp
===
--- clang/test/Parser/cxx-concepts-requires-clause.cpp
+++ clang/test/Parser/cxx-concepts-requires-clause.cpp
@@ -12,6 +12,7 @@
   struct AA;
   enum E : int;
   static int x;
+  static constexpr int z = 16;
 
   template  requires true
   void Mfoo();
@@ -24,6 +25,8 @@
 
   template  requires true
   using MQ = M;
+
+  constexpr int bazz() requires (z == 16);
 };
 
 template  requires (!0)
@@ -56,6 +59,9 @@
 template  requires true
 using Q = A;
 
+template requires (!0)
+constexpr int A::bazz() requires (z == 16) { return z; }
+
 struct C {
   template  requires true
   void Mfoo();
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -289,8 +289,14 @@
 
   LateParsedAttrList LateParsedAttrs(true);
   if (DeclaratorInfo.isFunctionDeclarator()) {
-if (Tok.is(tok::kw_requires))
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec &ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);
+  if (ScopeSpec.isValid() &&
+  Actions.ShouldEnterDeclaratorScope(getCurScope(), ScopeSpec))
+DeclScopeObj.EnterDeclaratorScope();
   ParseTrailingRequiresClause(DeclaratorInfo);
+}
 
 MaybeParseGNUAttributes(DeclaratorInfo, &LateParsedAttrs);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -167,8 +167,8 @@
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
 - Correctly defer dependent immediate function invocations until template instantiation.
   This fixes `GH55601 `_.
-
-
+- Class member variables are now in scope when parsing requires clauses. Fixes
+  `GH55216 `_.
 
 C++2b Feature Support
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

[Ivan Rodriguez via Phabricator via cfe-commits]

ivanrodriguez3753 added a comment.

This looks good to me, and I agree we should document what this is fixing. Any 
update on if/when this will land?

In my opinion, there's nothing broken about the user code (definitely 
contrived, though). They didn't ask for `stdbool.h` so there should not be a 
redeclaration error. This doesn't seem to be an issue when compiling for NVIDIA 
or host-only OpenMP, either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131639

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

[PATCH] D132503: [clang] Add cxx scope if needed for requires clause.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google added inline comments.



Comment at: clang/lib/Parse/ParseTemplate.cpp:293
+if (Tok.is(tok::kw_requires)) {
+  CXXScopeSpec ScopeSpec = DeclaratorInfo.getCXXScopeSpec();
+  DeclaratorScopeObj DeclScopeObj(*this, ScopeSpec);

erichkeane wrote:
> Not sure about this type, should this be a reference?
Mm, good point. Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132503

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

[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D131639#3749408 , 
@ivanrodriguez3753 wrote:

> This looks good to me, and I agree we should document what this is fixing. 
> Any update on if/when this will land?
>
> In my opinion, there's nothing broken about the user code (definitely 
> contrived, though). They didn't ask for `stdbool.h` so there should not be a 
> redeclaration error. This doesn't seem to be an issue when compiling for 
> NVIDIA or host-only OpenMP, either.

The example code in the bug report suggests the user is defining their own 
boolean type. I think the C and C++ standard is pretty clear that `__` and 
`_[A-Z]` identifiers are reserved and may clash at any point if defined by the 
user. This is typically why these are provided via headers that are guarded 
appropriates as `stdbool.h` is. I'm also not sure if this is a problem 
upstream, or it just reflects some vendor's implementation of it. Maybe others 
want to comment on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131639

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

[PATCH] D132661: [clang] Make guard(nocf) attribute available only for Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:373-375
   // Specifies Operating Systems for which the target applies, based off the
   // OSType enumeration in Triple.h
   list OSes;

Do we need customcode? Can we not use the OS list here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132661

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

[PATCH] D132659: PotentiallyEvaluatedContext in a ImmediateFunctionContext.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

This looks good to me but I want another set of eyes on this.




Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:942
+} // namespace GH51182
\ No newline at end of file


Please add a newline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132659

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

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 455626.
inclyc added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/FormatString.h
  clang/lib/AST/FormatString.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/FixIt/format.m
  clang/test/FixIt/format.mm
  clang/test/Sema/format-strings-freebsd.c
  clang/test/Sema/format-strings-scanf.c
  clang/test/Sema/format-strings.c
  clang/test/SemaObjC/format-strings-objc.m
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -819,13 +819,7 @@
 
   Unclear type relationship between a format specifier and its argument
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf";>N2562
-  
-Partial
-  Clang supports diagnostics checking format specifier validity, but
-  does not yet account for all of the changes in this paper, especially
-  regarding length modifiers like h and hh.
-
-  
+  Clang 16
 
 
 
Index: clang/test/SemaObjC/format-strings-objc.m
===
--- clang/test/SemaObjC/format-strings-objc.m
+++ clang/test/SemaObjC/format-strings-objc.m
@@ -191,7 +191,7 @@
   NSLog(@"%C", data);  // no-warning
 
   const wchar_t wchar_data = L'a';
-  NSLog(@"%C", wchar_data);  // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
+  NSLog(@"%C", wchar_data);  // no-warning
 }
 
 // Test that %@ works with toll-free bridging ().
@@ -273,7 +273,7 @@
 
 void testUnicode(void) {
   NSLog(@"%C", 0x2022); // no-warning
-  NSLog(@"%C", 0x202200); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
+  NSLog(@"%C", 0x202200); // no-warning
 }
 
 // Test Objective-C modifier flags.
Index: clang/test/Sema/format-strings.c
===
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -830,3 +830,64 @@
   printf_arg2("foo", "%s string %i\n", "aaa", 123);
   printf_arg2("%s string\n", "foo", "bar"); // expected-warning{{data argument not used by format string}}
 }
+
+void test_promotion(void) {
+  // Default argument promotions for *printf in N2562
+  // https://github.com/llvm/llvm-project/issues/57102
+  // N2562: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
+  int i;
+  signed char sc;
+  unsigned char uc;
+  char c;
+  short ss;
+  unsigned short us;
+  wchar_t wc;
+
+  printf("%hhd %hd %d %hhd %hd %d", i, i, i, sc, sc, sc); // no-warning
+  printf("%hhd %hd %d %hhd %hd %d", uc, uc, uc, c, c, c); // no-warning
+
+  // %ld %lld %llx
+  printf("%ld", i); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
+  printf("%lld", i); // expected-warning{{format specifies type 'long long' but the argument has type 'int'}}
+  printf("%ld", sc); // expected-warning{{format specifies type 'long' but the argument has type 'signed char'}}
+  printf("%lld", sc); // expected-warning{{format specifies type 'long long' but the argument has type 'signed char'}}
+  printf("%ld", uc); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned char'}}
+  printf("%lld", uc); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned char'}}
+  printf("%llx", i); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'int'}}
+
+  // ill formed spec for floats
+  printf("%hf", // expected-warning{{length modifier 'h' results in undefined behavior or no effect with 'f' conversion specifier}}
+  sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+
+  // for %hhd and `short` they are compatible by promotions but more likely misuse
+  printf("%hd", ss); // no-warning
+  printf("%hhd", ss); // expected-warning{{format specifies type 'char' but the argument has type 'short'}}
+  printf("%hu", us); // no-warning
+  printf("%hhu", ss); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'short'}}
+
+  // floats & integers are not compatible
+  printf("%f", i); // expected-warning{{format specifies type 'double' but the argument has type 'int'}}
+  printf("%f", sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+  printf("%f", uc); // expected-warning{{format specifies type 'double' but the argument has type 'unsigned char'}}
+  printf("%f", c); // expected-warning{{format specifies type 'double' but the argument has type 'char'}}
+  printf("%f", ss); // expected-warning{{format specifies type 'double' but the argu

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

[Rong Xu via Phabricator via cfe-commits]

xur added inline comments.



Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:606
+} else {
+  StaticFuncMap[NewName] = "---";
+}

davidxl wrote:
> define a macro for the marker string.
Will do.



Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:655
 const sampleprof::FunctionSamples &FS = PD.second;
 auto It = InstrProfileMap.find(FContext.toString());
+if (It == InstrProfileMap.end()) {

davidxl wrote:
> Skip to the next (using continue) when FS is not interesting (cold etc)
OK. 



Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:667
+}
+if (FS.getMaxCountInside() > ColdSampleThreshold &&
 It != InstrProfileMap.end() &&

davidxl wrote:
> continue when Itr == end or when Itr is not cold
Will change.


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

https://reviews.llvm.org/D132600

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

[PATCH] D132672: [Docs] [HLSL] Documenting HLSL Entry Functions

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: tex3d, pow2clk, bob80905, bogner, python3kgae, 
gracejennings.
Herald added a subscriber: Anastasia.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

This document describes the basic usage and implementation details for
HLSL entry functions in Clang.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132672

Files:
  clang/docs/HLSL/EntryFunctions.rst
  clang/docs/HLSL/HLSLDocs.rst


Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -12,3 +12,4 @@
:maxdepth: 1
 
ResourceTypes
+   EntryFunctions
Index: clang/docs/HLSL/EntryFunctions.rst
===
--- /dev/null
+++ clang/docs/HLSL/EntryFunctions.rst
@@ -0,0 +1,46 @@
+
+HLSL Entry Functions
+
+
+.. contents::
+   :local:
+
+Usage
+=
+
+In HLSL entry functions denote the starting point for shader execution. They
+must be known at compile time. For all non-library shaders, the compiler 
assumes
+the default entry function name ``main``, unless the DXC ``/E`` option is
+provided to specify an alternate entry point. For library shaders entry points
+are denoted using the ``[shader(...)]`` attribute.
+
+All scalar parameters to entry functions must have semantic annotations, and 
all
+struct parameters must have semantic annotations on every field in the struct
+declaration. Additionally if the entry function has a return type, a semantic
+annotation must be provided for the return type as well.
+
+HLSL entry functions can be called from other parts of the shader, which has
+implications on code generation.
+
+Implementation Details
+==
+
+In clang the DXC ``/E`` option is translated to the cc1 flag ``-hlsl-entry``,
+which in turn applies the ``HLSLShader`` attribute to the function with the
+specified name. This allows code generation for entry functions to always key
+off the presence of the ``HLSLShader`` attribute, regardless of what shader
+profile you are compiling.
+
+In code generation, two functions are generated. One is the user defined
+function, which is code generated as a mangled C++ function with internal
+linkage following normal function code generation.
+
+The actual exported entry function which can be called by the GPU driver is a
+``void(void)`` function that isn't name mangled. In code generation we generate
+the unmangled entry function, instantiations of the parameters with their
+semantic values populated, and a call to the user-defined function. After the
+call instruction the return value (if any) is saved via the appropriate
+intrinsic for its semantic annotation.
+
+During optimization all calls in HLSL are inlined, so this has no impact on
+final code.


Index: clang/docs/HLSL/HLSLDocs.rst
===
--- clang/docs/HLSL/HLSLDocs.rst
+++ clang/docs/HLSL/HLSLDocs.rst
@@ -12,3 +12,4 @@
:maxdepth: 1
 
ResourceTypes
+   EntryFunctions
Index: clang/docs/HLSL/EntryFunctions.rst
===
--- /dev/null
+++ clang/docs/HLSL/EntryFunctions.rst
@@ -0,0 +1,46 @@
+
+HLSL Entry Functions
+
+
+.. contents::
+   :local:
+
+Usage
+=
+
+In HLSL entry functions denote the starting point for shader execution. They
+must be known at compile time. For all non-library shaders, the compiler assumes
+the default entry function name ``main``, unless the DXC ``/E`` option is
+provided to specify an alternate entry point. For library shaders entry points
+are denoted using the ``[shader(...)]`` attribute.
+
+All scalar parameters to entry functions must have semantic annotations, and all
+struct parameters must have semantic annotations on every field in the struct
+declaration. Additionally if the entry function has a return type, a semantic
+annotation must be provided for the return type as well.
+
+HLSL entry functions can be called from other parts of the shader, which has
+implications on code generation.
+
+Implementation Details
+==
+
+In clang the DXC ``/E`` option is translated to the cc1 flag ``-hlsl-entry``,
+which in turn applies the ``HLSLShader`` attribute to the function with the
+specified name. This allows code generation for entry functions to always key
+off the presence of the ``HLSLShader`` attribute, regardless of what shader
+profile you are compiling.
+
+In code generation, two functions are generated. One is the user defined
+function, which is code generated as a mangled C++ function with internal
+linkage following normal function code generation.
+
+The actual exported entry function which can be called by the GPU driver is a
+``void(void)`` function that isn't name mangled. In code 

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D132568#3747971 , @inclyc wrote:

>> Do we want to encode that in `test_promotion` in 
>> `clang/test/Sema/format-strings.c`? Seems like tests on shorts are missing.
>
> Tests for short and char "incompatibility" could be found elsewhere in this 
> file.
>
> format-strings.c
>
>   void should_understand_small_integers(void) {
> printf("%hhu", (short) 10); // expected-warning{{format specifies type 
> 'unsigned char' but the argument has type 'short'}}
> printf("%hu\n", (unsigned char)1); // warning with -Wformat-pedantic only
> printf("%hu\n", (uint8_t)1);   // warning with -Wformat-pedantic only
>   }
>   /* ... */
>   void test13(short x) {
> char bel = 007;
> printf("bel: '0%hhd'\n", bel); // no-warning
> printf("x: '0%hhd'\n", x); // expected-warning {{format specifies type 
> 'char' but the argument has type 'short'}}
>   }
>
> Do I need to explicitly test again in the `test_promotion`?

Feel free to delete or move existing test cases if they make more sense to 
remain together in your added block, rather than more isolated in other test 
functions.




Comment at: clang/lib/AST/FormatString.cpp:401
+  if (const auto *BT = argTy->getAs()) {
+if (!Ptr) {
+  switch (BT->getKind()) {

aaron.ballman wrote:
> It's a bit strange that we have two switches over the same `BT->getKind()` 
> and the only difference is `!Ptr`; would it be easier to read if we combined 
> the two switches into one and had logic in the individual cases for `Ptr` vs 
> not `Ptr`?
I almost made the same recommendation myself.  For the below switch pair, and 
the pair above.



Comment at: clang/lib/AST/FormatString.cpp:408-410
+T == C.ShortTy || T == C.UnsignedShortTy) {
+  return MatchPromotion;
+}

nickdesaulniers wrote:
> `{}` aren't necessary
make sure to mark these done in phabricator UI



Comment at: clang/test/Sema/format-strings-scanf.c:271-272
+  scanf("%llx", &i); // expected-warning{{format specifies type 'unsigned long 
long *' but the argument has type 'int *'}}
+  scanf("%hf", // expected-warning{{length modifier 'h' results in undefined 
behavior or no effect with 'f' conversion specifier}}
+  &sc); // Is this a clang bug ?
+  scanf("%s", i); // expected-warning{{format specifies type 'char *' but the 
argument has type 'int'}}

aaron.ballman wrote:
> nickdesaulniers wrote:
> > Hmm...
> @nickdesaulniers -- are you thinking this might be a pedantic warning because 
> it's always valid to cast to a pointer to char and write into it? e.g.,
> ```
> short s;
> char *cp = (char *)&s;
> *cp = 0; // Not UB
> ```
No, was just curious about the comment `Is this a clang bug ?`. Looks like my 
comment has moved from the source location, and that you have a similar thread 
below. This one can be marked done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

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

[PATCH] D132659: PotentiallyEvaluatedContext in a ImmediateFunctionContext.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a subscriber: cor3ntin.
shafik added a comment.

CC @cor3ntin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132659

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

[PATCH] D132568: [clang][Sema] check default argument promotions for printf

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/lib/AST/FormatString.cpp:401
+  if (const auto *BT = argTy->getAs()) {
+if (!Ptr) {
+  switch (BT->getKind()) {

nickdesaulniers wrote:
> aaron.ballman wrote:
> > It's a bit strange that we have two switches over the same `BT->getKind()` 
> > and the only difference is `!Ptr`; would it be easier to read if we 
> > combined the two switches into one and had logic in the individual cases 
> > for `Ptr` vs not `Ptr`?
> I almost made the same recommendation myself.  For the below switch pair, and 
> the pair above.
> It's a bit strange that we have two switches over the same `BT->getKind()` 
> and the only difference is `!Ptr`; would it be easier to read if we combined 
> the two switches into one and had logic in the individual cases for `Ptr` vs 
> not `Ptr`?

These two switch pairs have different functions. The lower one is only 
responsible for checking whether there is a signed or unsigned integer, and the 
upper one is checking whether there is a promotion (or type confusing). Will 
they be more difficult to understand if they are written together? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132568

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

[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

[Ivan Rodriguez via Phabricator via cfe-commits]

ivanrodriguez3753 added a comment.

The user didn't define any `__` or `_[A-Z]` identifiers, though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131639

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

[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D131639#3749563 , 
@ivanrodriguez3753 wrote:

> The user didn't define any `__` or `_[A-Z]` identifiers, though?

Am I misunderstanding the test input?

  /* Visual Studio < 2013 does not have stdbool.h so here it is a replacement: 
*/
  #if defined __STDC__ && defined __STDC_VERSION__ && __STDC_VERSION__ >= 
199901L
  /* have a C99 compiler */
  typedef _Bool bool;
  #else
  /* do not have a C99 compiler */
  typedef unsigned char bool;
  #endif

I assumed this was the user defining this on their own.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131639

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

[PATCH] D132592: [Clang] Implement function attribute nouwtable

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

Thanks for taking a look!

In D132592#3749261 , @aaron.ballman 
wrote:

> Do we have any evidence that users need this level of control or will 
> understand how to properly use the attribute? The command line option makes 
> sense to me because it's an all-or-nothing flag, but I'm not certain I 
> understand the need for per-function control.

https://github.com/llvm/llvm-project/blob/064a08cd955019da9130f1109bfa534e79b8ec7c/llvm/include/llvm/IR/Function.h#L639-L641,
 per-function unwind table entry depends on both nounwind and uwtable. We have 
nothrow attribute for nounwind but not nouwtable for uwtable. With this, a user 
could use function attributes to control unwind table entry generation which 
could only be achieved by inline assembly or writing assembly files directly. 
I'd consider this a companion of nothrow. So making them both per-function 
attribute seems natural?

> Also, if a user gets this wrong (applies the attribute where they shouldn't), 
> what is the failure mode (does it introduce any exploitable behavior)?

The flag may only suppress unwind table emission instead of causing more, the 
lack of unwind table may only stop control flow rather than giving it more 
freedom. So I think this is safe from a security perspective. Using it wrong 
may cause unnecessary crashes just like any other memory bugs, but not a 
malicious binary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132592

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

[PATCH] D132600: [llvm-profdata] Handle internal linkage functions in profile supplementation

[Rong Xu via Phabricator via cfe-commits]

xur updated this revision to Diff 455640.
xur added a comment.

Integrated David's review commends.


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

https://reviews.llvm.org/D132600

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/include/llvm/ProfileData/SampleProf.h
  llvm/test/tools/llvm-profdata/Inputs/FUnique.afdotext
  llvm/test/tools/llvm-profdata/Inputs/FUnique.proftext
  llvm/test/tools/llvm-profdata/Inputs/NoFUnique.afdotext
  llvm/test/tools/llvm-profdata/Inputs/NoFUnique.proftext
  llvm/test/tools/llvm-profdata/suppl-instr-with-sample-static-func.test
  llvm/tools/llvm-profdata/llvm-profdata.cpp

Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -31,6 +31,7 @@
 #include "llvm/Support/Format.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/MD5.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ThreadPool.h"
@@ -42,6 +43,10 @@
 
 using namespace llvm;
 
+// We use this string to indicate that there are
+// multiple static functions map to the same name.
+const std::string DuplicateNameStr = "";
+
 enum ProfileFormat {
   PF_None = 0,
   PF_Text,
@@ -478,7 +483,7 @@
   ZeroCounterRatio = (float)ZeroCntNum / CntNum;
 }
 
-/// Either set all the counters in the instr profile entry \p IFE to -1
+// Either set all the counters in the instr profile entry \p IFE to -1
 /// in order to drop the profile or scale up the counters in \p IFP to
 /// be above hot threshold. We use the ratio of zero counters in the
 /// profile of a function to decide the profile is helpful or harmful
@@ -539,7 +544,73 @@
unsigned InstrProfColdThreshold) {
   // Function to its entry in instr profile.
   StringMap InstrProfileMap;
+  StringMap StaticFuncMap;
   InstrProfSummaryBuilder IPBuilder(ProfileSummaryBuilder::DefaultCutoffs);
+
+  auto checkSampleProfileHasFUnique = [&Reader]() {
+for (const auto &PD : Reader->getProfiles()) {
+  auto &FContext = PD.first;
+  if (FContext.toString().find(FunctionSamples::UniqSuffix) !=
+  std::string::npos) {
+return true;
+  }
+}
+return false;
+  };
+
+  bool SampleProfileHasFUnique = checkSampleProfileHasFUnique();
+
+  auto buildStaticFuncMap = [&StaticFuncMap,
+ SampleProfileHasFUnique](const StringRef &Name) {
+std::string Prefixes[] = {".cpp:", "cc:", ".c:", ".hpp:", ".h:"};
+size_t PrefixPos = StringRef::npos;
+for (auto &Prefix : Prefixes) {
+  PrefixPos = Name.find_insensitive(Prefix);
+  if (PrefixPos == StringRef::npos)
+continue;
+  PrefixPos += Prefix.size();
+  break;
+}
+
+if (PrefixPos == StringRef::npos) {
+  return;
+}
+
+StringRef NewName = Name.drop_front(PrefixPos);
+StringRef FName = Name.substr(0, PrefixPos - 1);
+if (NewName.size() == 0) {
+  return;
+}
+
+// This name should have a static linkage.
+size_t PostfixPos = NewName.find(FunctionSamples::UniqSuffix);
+bool ProfileHasFUnique = (PostfixPos != StringRef::npos);
+
+if (SampleProfileHasFUnique) {
+  // If profile also uses funqiue, nothing to do here.
+  // Otherwise, we need to build the map.
+  if (!ProfileHasFUnique) {
+std::string NStr =
+NewName.str() + getUniqueInternalLinkagePostfix(FName);
+NewName = StringRef(NStr);
+StaticFuncMap[NewName] = Name;
+return;
+  }
+} else {
+  // If profile does not use -funique-internal-linakge-symbols, nothing to
+  // do here. Otherwise, we need to trim.
+  if (ProfileHasFUnique) {
+NewName = NewName.substr(0, PostfixPos);
+  }
+}
+
+if (StaticFuncMap.find(NewName) == StaticFuncMap.end()) {
+  StaticFuncMap[NewName] = Name;
+} else {
+  StaticFuncMap[NewName] = DuplicateNameStr;
+}
+  };
+
   for (auto &PD : WC->Writer.getProfileData()) {
 // Populate IPBuilder.
 for (const auto &PDV : PD.getValue()) {
@@ -553,7 +624,9 @@
 
 // Initialize InstrProfileMap.
 InstrProfRecord *R = &PD.getValue().begin()->second;
-InstrProfileMap[PD.getKey()] = InstrProfileEntry(R);
+StringRef FullName = PD.getKey();
+InstrProfileMap[FullName] = InstrProfileEntry(R);
+buildStaticFuncMap(FullName);
   }
 
   ProfileSummary InstrPS = *IPBuilder.getSummary();
@@ -581,16 +654,27 @@
   // Find hot/warm functions in sample profile which is cold in instr profile
   // and adjust the profiles of those functions in the instr profile.
   for (const auto &PD : Reader->getProfiles()) {
-auto &FContext = PD.first;
 const sampleprof::FunctionSamples &FS = PD.second;
+if (FS.getMaxCountInside() <= ColdSampleThreshold ||
+FS.getBodySamples().size() < SupplMinSizeThreshold)
+ 

[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

[Ivan Rodriguez via Phabricator via cfe-commits]

ivanrodriguez3753 added a comment.

They are defining their own `bool`, which aliases to the built-in `_Bool` 
(which is reserved, as you noted with `_[A-Z]`). I thought `bool` was fair game 
unless they included `stdbool.h`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131639

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

[PATCH] D132659: PotentiallyEvaluatedContext in a ImmediateFunctionContext.

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

Yes, this looks good to me too. Thanks for working on this. 
Can you modify ReleaseNotes.rst to mention the fixed issue?




Comment at: clang/include/clang/Sema/Sema.h:1355
+  InImmediateFunctionContext) ||
+ // [expr.const#def:immediate_function_context]
+ // An expression or conversion is in an immediate function





Comment at: clang/lib/Sema/SemaDecl.cpp:14774
   if (!isLambdaCallOperator(FD))
+// [expr.const#def:immediate_function_context]
+// An expression or conversion is in an immediate function context if it is




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132659

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

[PATCH] D132659: PotentiallyEvaluatedContext in a ImmediateFunctionContext.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: cor3ntin, aaron.ballman, clang-language-wg.
aaron.ballman added a comment.

Adding a few more reviewers just in case, but this is generally looking good. 
Please also add a release note for the fix!




Comment at: clang/include/clang/Sema/Sema.h:1355
+  InImmediateFunctionContext) ||
+ // [expr.const#def:immediate_function_context]
+ // An expression or conversion is in an immediate function

cor3ntin wrote:
> 




Comment at: clang/include/clang/Sema/Sema.h:1357-1359
+ // context if it is potentially evaluated and either: its
+ // innermost enclosing non-block scope is a function parameter
+ // scope of an immediate function.





Comment at: clang/lib/Sema/SemaDecl.cpp:14774
   if (!isLambdaCallOperator(FD))
+// [expr.const#def:immediate_function_context]
+// An expression or conversion is in an immediate function context if it is

cor3ntin wrote:
> 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132659

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