[PATCH] D132030: [analyzer] Pass correct bldrCtx to computeObjectUnderConstruction

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Beautiful find, thanks!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:753
 
-SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts, Idx);
+SVal V = computeObjectUnderConstruction(E, State, currBldrCtx, LCtx, CC,
+CallOpts, Idx);

You probably want an updated builder context here as well. This function should 
be a simple wrapper, it should be completely interchangeable with calling both 
functions directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132030

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

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

[Alex Brachet via Phabricator via cfe-commits]

abrachet added a comment.

While digging around failures we were seeing related to this I found some 
behavior that diverges from gcc. See https://godbolt.org/z/qe18Wh4jf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117616

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

[PATCH] D132030: [analyzer] Pass correct bldrCtx to computeObjectUnderConstruction

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:753
 
-SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts, Idx);
+SVal V = computeObjectUnderConstruction(E, State, currBldrCtx, LCtx, CC,
+CallOpts, Idx);

NoQ wrote:
> You probably want an updated builder context here as well. This function 
> should be a simple wrapper, it should be completely interchangeable with 
> calling both functions directly.
Could you please elaborate more? I would see a reason to create a context here 
if I would expect that `currBlrdCtx` refers to a different `Block` that we want 
to perform construction in. And there is no indication on another `Block` being 
inplay here, and I would construct `NodeBlockCtx` with same block as 
`currBldrCtx`.
In other words,  I expect this function to be `handeConstructionContext` in 
current `Block`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132030

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

[PATCH] D130788: [clang-repl] Disable building when LLVM_STATIC_LINK_CXX_STDLIB is ON.

[Sunho Kim via Phabricator via cfe-commits]

sunho added a comment.

In D130788#3730533 , @sbc100 wrote:

> I'm not totally sure but I think the change is responsible for the emscripten 
> integration bot failing `InterpreterTest.CatchException`: 
> https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8807160007692150337/+/u/LLVM_regression/stdout
>
>   [==] Running 1 test from 1 test suite.
>   [--] Global test environment set-up.
>   [--] 1 test from InterpreterTest
>   [ RUN  ] InterpreterTest.CatchException
>   JIT session error: Symbols not found: [ __gxx_personality_v0, 
> _ZSt9terminatev, _ZTVN10__cxxabiv117__class_type_infoE, 
> __cxa_allocate_exception, __cxa_begin_catch, __cxa_end_catch, 
> __cxa_free_exception, __cxa_throw ]
>   Failure value returned from cantFail wrapped call
>   Failed to materialize symbols: { (main, { _ZN16custom_exceptionC2EPKc, 
> __clang_call_terminate, _ZTI16custom_exception, _ZTS16custom_exception, 
> throw_exception }) }
>   UNREACHABLE executed at 
> /b/s/w/ir/cache/builder/emscripten-releases/llvm-project/llvm/include/llvm/Support/Error.h:786!
>   Stack dump without symbol names (ensure you have llvm-symbolizer in your 
> PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x2464413)[0x55cb14660413]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246236c)[0x55cb1465e36c]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x24648df)[0x55cb146608df]
>   /lib/x86_64-linux-gnu/libpthread.so.0(+0x12980)[0x7fad2fab1980]
>   /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7fad2eb0de87]
>   /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7fad2eb0f7f1]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x23f798f)[0x55cb145f398f]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9de35)[0x55cb14099e35]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9d597)[0x55cb14099597]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246d6be)[0x55cb146696be]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246e659)[0x55cb1466a659]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246ee40)[0x55cb1466ae40]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x247b2c3)[0x55cb146772c3]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x247ab42)[0x55cb14676b42]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246559c)[0x55cb1466159c]
>   /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7fad2eaf0c87]
>   
> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9ceda)[0x55cb14098eda]
>
> This started happening consistently after this change 
> https://chromium.googlesource.com/emscripten-releases/+/584b2f531314d1e70cd5ebadcce7e015a6215c9a.
>   The only CL in that list that looks related seems to be this one.

Could you share the detailed build configuration of those bots?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130788

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

[PATCH] D131618: [WIP][Do NOT review] LLD related changes for -ffat-lto-objects support

[Arda Unal via Phabricator via cfe-commits]

arda updated this revision to Diff 453550.
arda added a comment.

Use obj2yaml and yaml2obj to avoid unreadable object files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131618

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/CodeGen/BackendUtil.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/ToolChain.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  lld/ELF/Config.h
  lld/ELF/Driver.cpp
  lld/ELF/InputFiles.cpp
  lld/ELF/Options.td
  lld/test/ELF/fatlto/Inputs/a-fatLTO.yaml
  lld/test/ELF/fatlto/Inputs/a-thinLTO.ll
  lld/test/ELF/fatlto/Inputs/a.c
  lld/test/ELF/fatlto/Inputs/a.h
  lld/test/ELF/fatlto/Inputs/a.yaml
  lld/test/ELF/fatlto/Inputs/main-fatLTO.yaml
  lld/test/ELF/fatlto/Inputs/main-thinLTO.ll
  lld/test/ELF/fatlto/Inputs/main.c
  lld/test/ELF/fatlto/Inputs/main.yaml
  lld/test/ELF/fatlto/fatlto.test
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Object/ObjectFile.cpp

Index: llvm/lib/Object/ObjectFile.cpp
===
--- llvm/lib/Object/ObjectFile.cpp
+++ llvm/lib/Object/ObjectFile.cpp
@@ -79,7 +79,7 @@
 bool ObjectFile::isSectionBitcode(DataRefImpl Sec) const {
   Expected NameOrErr = getSectionName(Sec);
   if (NameOrErr)
-return *NameOrErr == ".llvmbc";
+return *NameOrErr == ".llvmbc" || *NameOrErr == ".llvm.lto";
   consumeError(NameOrErr.takeError());
   return false;
 }
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -4953,6 +4953,37 @@
   llvm_unreachable("Unimplemented ObjectFormatType");
 }
 
+static const char *getSectionNameForBitcodeForFatLTO(const Triple &T) {
+  switch (T.getObjectFormat()) {
+  case Triple::MachO:
+llvm::report_fatal_error("MachO is not yet implemented for FatLTO");
+break;
+  case Triple::COFF:
+llvm::report_fatal_error("COFF is not yet implemented for FatLTO");
+break;
+  case Triple::ELF:
+return ".llvm.lto";
+  case Triple::Wasm:
+llvm::report_fatal_error("Wasm is not yet implemented for FatLTO");
+break;
+  case Triple::UnknownObjectFormat:
+return ".llvm.lto";
+  case Triple::GOFF:
+llvm::report_fatal_error("GOFF is not yet implemented for FatLTO");
+break;
+  case Triple::SPIRV:
+llvm::report_fatal_error("SPIRV is not yet implemented for FatLTO");
+break;
+  case Triple::XCOFF:
+llvm::report_fatal_error("XCOFF is not yet implemented for FatLTO");
+break;
+  case Triple::DXContainer:
+llvm::report_fatal_error("DXContainer is not yet implemented for FatLTO");
+break;
+  }
+  llvm_unreachable("Unimplemented ObjectFormatType");
+}
+
 void llvm::embedBitcodeInModule(llvm::Module &M, llvm::MemoryBufferRef Buf,
 bool EmbedBitcode, bool EmbedCmdline,
 const std::vector &CmdArgs) {
@@ -5045,3 +5076,68 @@
   llvm::ConstantArray::get(ATy, UsedArray), "llvm.compiler.used");
   NewUsed->setSection("llvm.metadata");
 }
+
+void llvm::embedBitcodeInFatObject(llvm::Module &M, llvm::MemoryBufferRef Buf) {
+  // Save llvm.compiler.used and remove it.
+  SmallVector UsedArray;
+  SmallVector UsedGlobals;
+  Type *UsedElementType = Type::getInt8Ty(M.getContext())->getPointerTo(0);
+  GlobalVariable *Used = collectUsedGlobalVariables(M, UsedGlobals, true);
+  for (auto *GV : UsedGlobals) {
+if (GV->getName() != "llvm.embedded.module" &&
+GV->getName() != "llvm.cmdline")
+  UsedArray.push_back(
+  ConstantExpr::getPointerBitCastOrAddrSpaceCast(GV, UsedElementType));
+  }
+  if (Used)
+Used->eraseFromParent();
+
+  // Embed the bitcode for the llvm module.
+  std::string Data;
+  ArrayRef ModuleData;
+  Triple T(M.getTargetTriple());
+
+  if (Buf.getBufferSize() == 0 ||
+  !isBitcode((const unsigned char *)Buf.getBufferStart(),
+ (const unsigned char *)Buf.getBufferEnd())) {
+// If the input is LLVM Assembly, bitcode is produced by serializing
+// the module. Use-lists order need to be preserved in this case.
+llvm::raw_string_ostream OS(Data);
+llvm::WriteBitcodeToFile(M, OS, /* ShouldPreserveUseListOrder */ true);
+ModuleData =
+ArrayRef((const uint8_t *)OS.str().data(), OS.str().size());
+  } else
+// If the input is LLVM bitcode, write the input byte stream directly.
+ModuleData = ArrayRef((const uint8_t *)Buf.getBufferStart(),
+   Buf.getBufferSize());
+  llvm::Constant *ModuleConstant =
+  llvm::ConstantDataArray::get(M.getContext(), ModuleData);
+  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+  M, Modul

[PATCH] D132031: Do not evaluate dependent immediate invocations

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

A few NITS for the release notes, otherwise LG




Comment at: clang/docs/ReleaseNotes.rst:162
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
+- Correctly defer dependent immediate invocations until template instantiation.
+  This fixes `Issue 55601 
`_.

NIT: a small nit



Comment at: clang/docs/ReleaseNotes.rst:163
+- Correctly defer dependent immediate invocations until template instantiation.
+  This fixes `Issue 55601 
`_.
 

NIT: to align with release notes above


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132031

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

[PATCH] D132030: [analyzer] Pass correct bldrCtx to computeObjectUnderConstruction

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:753
 
-SVal V = computeObjectUnderConstruction(E, State, LCtx, CC, CallOpts, Idx);
+SVal V = computeObjectUnderConstruction(E, State, currBldrCtx, LCtx, CC,
+CallOpts, Idx);

tomasz-kaminski-sonarsource wrote:
> NoQ wrote:
> > You probably want an updated builder context here as well. This function 
> > should be a simple wrapper, it should be completely interchangeable with 
> > calling both functions directly.
> Could you please elaborate more? I would see a reason to create a context 
> here if I would expect that `currBlrdCtx` refers to a different `Block` that 
> we want to perform construction in. And there is no indication on another 
> `Block` being inplay here, and I would construct `NodeBlockCtx` with same 
> block as `currBldrCtx`.
> In other words,  I expect this function to be `handeConstructionContext` in 
> current `Block`. 
Or to say it differently, I expect `BldCtx` not being `currBldrCtx` to be an 
unusual situation, that is limited to the construction of return value. Thus 
having it in `convenience` would only make it more currbesome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132030

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

[PATCH] D132109: [analyzer] Dump the environment entry kind as well

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

steakhal created this revision.
steakhal added a reviewer: NoQ.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a reviewer: Szelethus.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

By this change the `exploded-graph-rewriter` will display the class kind
of the expression of the environment entry. It makes easier to decide if
the given entry corresponds to the lvalue or to the rvalue of some
expression.

It turns out the rewriter already had support for visualizing it, but
probably was never actually used?

Example rewritten-dump after my change:
F2419: image.png 
Note the same //pretty// representation; now it's clear at first glance which 
is which.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132109

Files:
  clang/lib/StaticAnalyzer/Core/Environment.cpp
  clang/test/Analysis/expr-inspection.c


Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -36,7 +36,7 @@
 // CHECK-NEXT:   ]},
 // CHECK-NEXT:   "environment": { "pointer": "{{0x[0-9a-f]+}}", "items": [
 // CHECK-NEXT: { "lctx_id": {{[0-9]+}}, "location_context": "#0 Call", 
"calling": "foo", "location": null, "items": [
-// CHECK-NEXT:   { "stmt_id": {{[0-9]+}}, "pretty": 
"clang_analyzer_printState", "value": "&code{clang_analyzer_printState}" }
+// CHECK-NEXT:   { "stmt_id": {{[0-9]+}}, "kind": "ImplicitCastExpr", 
"pretty": "clang_analyzer_printState", "value": 
"&code{clang_analyzer_printState}" }
 // CHECK-NEXT: ]}
 // CHECK-NEXT:   ]},
 // CHECK-NEXT:   "constraints": [
Index: clang/lib/StaticAnalyzer/Core/Environment.cpp
===
--- clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -274,7 +274,8 @@
 
   const Stmt *S = I->first.getStmt();
   Indent(Out, InnerSpace, IsDot)
-  << "{ \"stmt_id\": " << S->getID(Ctx) << ", \"pretty\": ";
+  << "{ \"stmt_id\": " << S->getID(Ctx) << ", \"kind\": \""
+  << S->getStmtClassName() << "\", \"pretty\": ";
   S->printJson(Out, nullptr, PP, /*AddQuotes=*/true);
 
   Out << ", \"value\": ";


Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -36,7 +36,7 @@
 // CHECK-NEXT:   ]},
 // CHECK-NEXT:   "environment": { "pointer": "{{0x[0-9a-f]+}}", "items": [
 // CHECK-NEXT: { "lctx_id": {{[0-9]+}}, "location_context": "#0 Call", "calling": "foo", "location": null, "items": [
-// CHECK-NEXT:   { "stmt_id": {{[0-9]+}}, "pretty": "clang_analyzer_printState", "value": "&code{clang_analyzer_printState}" }
+// CHECK-NEXT:   { "stmt_id": {{[0-9]+}}, "kind": "ImplicitCastExpr", "pretty": "clang_analyzer_printState", "value": "&code{clang_analyzer_printState}" }
 // CHECK-NEXT: ]}
 // CHECK-NEXT:   ]},
 // CHECK-NEXT:   "constraints": [
Index: clang/lib/StaticAnalyzer/Core/Environment.cpp
===
--- clang/lib/StaticAnalyzer/Core/Environment.cpp
+++ clang/lib/StaticAnalyzer/Core/Environment.cpp
@@ -274,7 +274,8 @@
 
   const Stmt *S = I->first.getStmt();
   Indent(Out, InnerSpace, IsDot)
-  << "{ \"stmt_id\": " << S->getID(Ctx) << ", \"pretty\": ";
+  << "{ \"stmt_id\": " << S->getID(Ctx) << ", \"kind\": \""
+  << S->getStmtClassName() << "\", \"pretty\": ";
   S->printJson(Out, nullptr, PP, /*AddQuotes=*/true);
 
   Out << ", \"value\": ";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added a comment.

I would like to see this land. This also fixes 
https://github.com/llvm/llvm-project/issues/57046.

Are there any pending concerns of the reviewers which we need to address ? 
Happy to help in any way to move this patch forward.

CC: @erichkeane @cor3ntin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D131547: [Clang][AArch64] Use generic extract/insert vector for svget/svset/svcreate tuples

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

> Is there a formal requirement that LLVM must remain backward compatible with 
> older LLVM IR (beyond the target-independent parts)?

We have always done it in the past, and I don't see a good reason to change. 
This change is essentially for llvm 16, so we are talking about any bitcode 
between when SVE was added and that release. It is hard to tell how people will 
use bitcode up to that point and if they will expect it to continue working 
going forward. I think it's simpler to just add the upgrade code, than to try 
and argue that it is unneeded. But the upgrade code is really needed in D131548 
 (and D131687 
) where the old intrinsics are being removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131547

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

[PATCH] D126534: [analyzer] Deadstore static analysis: Fix false positive on C++17 assignments

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

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

It feels like there is no objection. I think it looks great and you did 
everything you could to address the review comments.
I believe we should not postpone this any further.
Consider committing this tomorrow, unless in the meantime @NoQ objects.


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

https://reviews.llvm.org/D126534

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

[clang] 6741f0f - Revert "[Driver] Support libc++ in MSVC"

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2022-08-18T08:22:43Z
New Revision: 6741f0f9124dd28e1a4bade173774fc47fb75912

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

LOG: Revert "[Driver] Support libc++ in MSVC"

This reverts commit a4230319f7af786f4d947f39a870f339b32a6fe7.

This is causing build failures in the 2-stage build.

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/MSVC.cpp

Removed: 
clang/test/Driver/Inputs/msvc_libcxx_tree/usr/bin/.keep
clang/test/Driver/Inputs/msvc_libcxx_tree/usr/include/c++/v1/.keep

clang/test/Driver/Inputs/msvc_libcxx_tree/usr/include/x86_64-pc-windows-msvc/c++/v1/.keep
clang/test/Driver/Inputs/msvc_libcxx_tree/usr/lib/.keep

clang/test/Driver/Inputs/msvc_libcxx_tree/usr/lib/x86_64-pc-windows-msvc/.keep
clang/test/Driver/msvc-libcxx.cpp



diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 216560cfc86ec..8c9901d086eee 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4017,12 +4017,12 @@ def noprofilelib : Flag<["-"], "noprofilelib">;
 def noseglinkedit : Flag<["-"], "noseglinkedit">;
 def nostartfiles : Flag<["-"], "nostartfiles">, Group;
 def nostdinc : Flag<["-"], "nostdinc">, Flags<[CoreOption]>;
-def nostdlibinc : Flag<["-"], "nostdlibinc">, Flags<[CoreOption]>;
-def nostdincxx : Flag<["-"], "nostdinc++">, Flags<[CC1Option, CoreOption]>,
+def nostdlibinc : Flag<["-"], "nostdlibinc">;
+def nostdincxx : Flag<["-"], "nostdinc++">, Flags<[CC1Option]>,
   HelpText<"Disable standard #include directories for the C++ standard 
library">,
   MarshallingInfoNegativeFlag>;
 def nostdlib : Flag<["-"], "nostdlib">, Group;
-def nostdlibxx : Flag<["-"], "nostdlib++">, Group;
+def nostdlibxx : Flag<["-"], "nostdlib++">;
 def object : Flag<["-"], "object">;
 def o : JoinedOrSeparate<["-"], "o">, Flags<[NoXarchOption, RenderAsInput,
   CC1Option, CC1AsOption, FC1Option, FlangOption]>,

diff  --git a/clang/lib/Driver/ToolChains/MSVC.cpp 
b/clang/lib/Driver/ToolChains/MSVC.cpp
index e8d6178fdfa18..14ebe38ee1918 100644
--- a/clang/lib/Driver/ToolChains/MSVC.cpp
+++ b/clang/lib/Driver/ToolChains/MSVC.cpp
@@ -153,11 +153,6 @@ void visualstudio::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
   if (!C.getDriver().IsCLMode() && Args.hasArg(options::OPT_L))
 for (const auto &LibPath : Args.getAllArgValues(options::OPT_L))
   CmdArgs.push_back(Args.MakeArgString("-libpath:" + LibPath));
-  // Add library directories for standard library shipped with the toolchain.
-  for (const auto &LibPath : TC.getFilePaths()) {
-if (TC.getVFS().exists(LibPath))
-  CmdArgs.push_back(Args.MakeArgString("-libpath:" + LibPath));
-  }
 
   CmdArgs.push_back("-nologo");
 
@@ -746,36 +741,7 @@ void MSVCToolChain::AddClangSystemIncludeArgs(const 
ArgList &DriverArgs,
 
 void MSVCToolChain::AddClangCXXStdlibIncludeArgs(const ArgList &DriverArgs,
  ArgStringList &CC1Args) const 
{
-  if (DriverArgs.hasArg(options::OPT_nostdlibinc) ||
-  DriverArgs.hasArg(options::OPT_nostdincxx))
-return;
-
-  switch (GetCXXStdlibType(DriverArgs)) {
-  case ToolChain::CST_Libcxx: {
-SmallString<128> P(getDriver().Dir);
-llvm::sys::path::append(P, "..", "include");
-
-std::string Version = detectLibcxxVersion(P);
-if (Version.empty())
-  return;
-
-// First add the per-target include path if it exists.
-SmallString<128> TargetDir(P);
-llvm::sys::path::append(TargetDir, getTripleString(), "c++", Version);
-if (getVFS().exists(TargetDir))
-  addSystemInclude(DriverArgs, CC1Args, TargetDir);
-
-// Second add the generic one.
-SmallString<128> Dir(P);
-llvm::sys::path::append(Dir, "c++", Version);
-addSystemInclude(DriverArgs, CC1Args, Dir);
-break;
-  }
-
-  default:
-// TODO: Shall we report an error for other C++ standard libraries?
-break;
-  }
+  // FIXME: There should probably be logic here to find libc++ on Windows.
 }
 
 VersionTuple MSVCToolChain::computeMSVCVersion(const Driver *D,

diff  --git a/clang/test/Driver/Inputs/msvc_libcxx_tree/usr/bin/.keep 
b/clang/test/Driver/Inputs/msvc_libcxx_tree/usr/bin/.keep
deleted file mode 100644
index e69de29bb2d1d..0

diff  --git 
a/clang/test/Driver/Inputs/msvc_libcxx_tree/usr/include/c++/v1/.keep 
b/clang/test/Driver/Inputs/msvc_libcxx_tree/usr/include/c++/v1/.keep
deleted file mode 100644
index e69de29bb2d1d..0

diff  --git 
a/clang/test/Driver/Inputs/msvc_libcxx_tree/usr/include/x86_64-pc-windows-msvc/c++/v1/.keep
 
b/clang/test/Driver/Inputs/msvc_libcxx_tree/usr/include/x86_64-pc-windows-msvc/c++/v1/.keep
deleted file mode 100644
index e69de2

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang-tools-extra.

This brings IncludeCleaner's reference discovery from AST to the parity
with current implementation in clangd. Some highlights:

- Handling of MemberExprs, only the member declaration is marked as referenced 
and not the container, unlike clangd.
- Constructor calls, only the constructor and not the container, unlike clangd.
- All the possible candidates for unresolved overloads, same as clangd.
- All the shadow decls for using-decls, same as clangd.
- Declarations for definitions of enums with an underlying type and functions, 
same as clangd.
- Using typelocs, using templatenames and typedefs only reference the found 
decl, same as clangd.
- Template specializations only reference the primary template, not the 
explicit specializations, to be fixed.
- Expr types aren't marked as used, unlike clangd.

Going forward, we can consider having signals to indicate type of a
reference (e.g. `implicit` signal for type of an expr) so that the
applications can perform a filtering based on their needs.
At the moment the biggest discrepancy is around type of exprs, i.e. not
marking containers for member/constructor accesses. I believe this is
the right model since the declaration of the member and the container
should be available in a single file (modulo macros).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132110

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -25,6 +25,7 @@
   Inputs.ExtraFiles["target.h"] = Target.code().str();
   Inputs.ExtraArgs.push_back("-include");
   Inputs.ExtraArgs.push_back("target.h");
+  Inputs.ExtraArgs.push_back("-std=c++17");
   TestAST AST(Inputs);
   const auto &SM = AST.sourceManager();
 
@@ -84,6 +85,8 @@
   testWalk("struct S { static int ^x; };", "int y = S::^x;");
   // Canonical declaration only.
   testWalk("extern int ^x; int x;", "int y = ^x;");
+  // Return type of `foo` isn't used.
+  testWalk("struct S{}; S ^foo();", "auto bar() { return ^foo(); }");
 }
 
 TEST(WalkAST, TagType) {
@@ -98,11 +101,64 @@
 using ns::^x;
   )cpp",
"int y = ^x;");
+  testWalk("using ^foo = int;", "^foo x;");
+  testWalk("struct S {}; using ^foo = S;", "^foo x;");
+}
+
+TEST(WalkAST, Using) {
+  testWalk("namespace ns { void ^x(); void ^x(int); }", "using ns::^x;");
+  testWalk("namespace ns { struct S; } using ns::^S;", "^S *s;");
+}
+
+TEST(WalkAST, Namespaces) {
+  testWalk("namespace ns { void x(); }", "using namespace ^ns;");
+}
+
+TEST(WalkAST, TemplateNames) {
+  testWalk("template struct ^S {};", "^S s;");
+  // FIXME: Template decl has the wrong primary location for type-alias template
+  // decls.
   testWalk(R"cpp(
-namespace ns { struct S; } // Not used
-using ns::S; // FIXME: S should be used
-  )cpp",
-   "^S *x;");
+  template  struct S {};
+  template  ^using foo = S;)cpp",
+   "^foo x;");
+  testWalk(R"cpp(
+  namespace ns {template  struct S {}; }
+  using ns::^S;)cpp",
+   "^S x;");
+  testWalk("template struct ^S {};",
+   R"cpp(
+  template  typename> struct X {};
+  X<^S> x;)cpp");
+  testWalk("template struct ^S { S(T); };", "^S s(42);");
+  // Should we mark the specialization instead?
+  testWalk("template struct ^S {}; template <> struct S {};",
+   "^S s;");
+}
+
+TEST(WalkAST, MemberExprs) {
+  testWalk("struct S { void ^foo(); };", "void foo() { S{}.^foo(); }");
+}
+
+TEST(WalkAST, ConstructExprs) {
+  testWalk("struct ^S {};", "S ^t;");
+  testWalk("struct S { ^S(int); };", "S ^t(42);");
+}
+
+TEST(WalkAST, Functions) {
+  // Definition uses declaration, not the other way around.
+  testWalk("void ^foo();", "void ^foo() {}");
+  testWalk("void foo() {}", "void ^foo();");
+
+  // Unresolved calls marks all the overloads.
+  testWalk("void ^foo(int); void ^foo(char);",
+   "template  void bar() { ^foo(T{}); }");
+}
+
+TEST(WalkAST, Enums) {
+  testWalk("enum E { ^A = 42, B = 43 };", "int e = ^A;");
+  testWalk("enum class ^E : int;", "enum class ^E : int {};");
+  testWalk("enum class E : int {};", "enum class ^E : int ;");
 }
 
 } // namespace
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -7,7 +7,17 @@
 //===--===//
 
 #incl

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

This looks good to me.

I *think* landing things without doing silent sfinae for incomplete types is 
okay-ish, but I'd like to here from other folks.

@royjacobson Have you looked into that? Do you know how involved would it be?

Either way, we should make sure that if that bit is dealt with separately, we 
have an issue or something to track it, maybe a note in `cxx_status`.




Comment at: clang/docs/ReleaseNotes.rst:494-497
+- Implemented "Conditionally Trivial Special Member Functions" (`P0848 
`_).
+  Note: The handling of deleted functions is not yet compliant, as Clang
+  does not implement `DR1496 
`_
+  and `DR1734 
`_.

royjacobson wrote:
> h-vetinari wrote:
> > Is that lack of compliance worth a note in `cxx_status`?
> I'm not very opinionated about this, but I tend to not significant enough. I 
> mean, 7 years later and only MSVC have even bothered to implement them.
We might has well, I think it's a good way to not lose track of it.



Comment at: clang/lib/Sema/SemaDecl.cpp:17875
+return true;
+  if (!Context.hasSameType(M1->getParamDecl(0)->getType(),
+   M2->getParamDecl(0)->getType()))

royjacobson wrote:
> royjacobson wrote:
> > shafik wrote:
> > > What happens if we have further parameters with default arguments? Unless 
> > > I am missing something they are still special member functions but the 
> > > proposal does not seem to cover them.
> > That's an excellent question.
> > 
> > I'm not sure what to do about default arguments. In a context where the 
> > additional parameters matter, you're not using them as constructors 
> > anymore, right? So why would this affect the type traits?
> > On the one hand [over.match.best] is against this idea of comparing 
> > constraints when the parameters differ. So also every context where this 
> > actually matters the overload resolution would probably be ambiguous anyway?
> > 
> > @BRevzin, what do you think? Is the wording intentional to include 
> > copy/move constructors with default arguments as well?
> > 
> > I checked with GCC and they seem to handle default arguments separately: 
> > https://godbolt.org/z/1ch3M7MjP
> > 
> FWIW, I filed https://github.com/cplusplus/CWG/issues/110
> 
> I'm not on the reflector myself, so I don't know if there's been a follow-up 
> there.
Yes, i think it's reasonable to not concerns ourselves with default parameters 
until wg21 decides otherwise



Comment at: clang/test/SemaCXX/constrained-special-member-functions.cpp:200
+// FIXME: We should not throw an error, instead SFINAE should make the 
constraint
+// silently unsatisfied. See [temp.constr.constr]p5
+template 

Have you been able to investigate that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

___
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]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I wanted to do only pointers here, but they are impossible to test without 
having some support for DeclRefExprs.

This also implements assignments because that was broken when implementing 
DeclRefExprs. Assignments were handled through `LValueToRValue` casts before.


Repository:
  rG LLVM Github Monorepo

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
@@ -32,3 +32,12 @@
 static_assert(!0, "");
 static_assert(-true, "");
 static_assert(-false, ""); //expected-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;
+}
Index: clang/test/AST/Interp/cxx20.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/cxx20.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s -DREFERENCE
+
+
+// expected-no-diagnostics
+constexpr int getMinus5() {
+  int a = 10;
+  a = -5;
+  int *p = &a;
+  return *p;
+}
+
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
@@ -98,11 +98,15 @@
   // Value loaded - nothing to do here.
   return true;
 },
-[this, CE](PrimType T) {
-  // Pointer on stack - dereference it.
-  if (!this->emitLoadPop(T, CE))
-return false;
-  return DiscardResult ? this->emitPop(T, CE) : true;
+[this, CE, SubExpr](PrimType T) {
+  // DeclRefExpr are modeled as pointers, so nothing to do.
+  if (isa(SubExpr->IgnoreImpCasts())) {
+// Pointer on stack - dereference it.
+if (!this->emitLoadPop(T, CE))
+  return false;
+return DiscardResult ? this->emitPop(T, CE) : true;
+  }
+  return true;
 });
   }
 
@@ -209,6 +213,12 @@
   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;
+  // TODO: Assignments return the assigned value, so the pop() here
+  //   should proably just go away.
+  return this->emitPopPtr(BO);
 default:
   return this->bail(BO);
 }
@@ -596,8 +606,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++
@@ -607,16 +616,37 @@
 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("Referencing requires a pointer");
+  return false;
+},
+[this, E](PrimType T) {
+  // T: type we get here from classify() is the subexpr
+  // TODO: Is this right?
+  T = *classify(E->getType());
+  if (!this->emitLoadPop(T, E))
+return false;
+  return DiscardResult ? this->emitPop(T, E) : true;
+});
   case UO_Not:// ~x
   case UO_Real:   // __real x
   case UO_Imag:   // __imag x
@@ -628,6 +658,24 @@
   return false;
 }
 
+template 
+bool ByteCodeExprGen:

[PATCH] D132031: Do not evaluate dependent immediate invocations

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 453562.
usaxena95 marked 2 inline comments as done.
usaxena95 added a comment.

Updated release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132031

Files:
  clang/docs/ReleaseNotes.rst
  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
@@ -648,9 +648,40 @@
 
 static_assert(bar<15>() == 15);
 static_assert(baz() == sizeof(int));
-
 } // namespace value_dependent
 
+// https://github.com/llvm/llvm-project/issues/55601
+namespace issue_55601 {
+template
+class Bar {
+  consteval static T x() { return 5; }  // expected-note {{non-constexpr 
constructor 'derp' cannot be used in a constant expression}}
+ public:
+  Bar() : a(x()) {} // expected-error {{call to consteval function 
'issue_55601::Bar::x' is not a constant expression}}
+// expected-error@-1 {{call to consteval function 
'issue_55601::derp::operator int' is not a constant expression}}
+// expected-note@-2 {{in call to 'x()'}}
+// expected-note@-3 {{non-literal type 'issue_55601::derp' 
cannot be used in a constant expression}}
+ private:
+  int a;
+};
+Bar f;
+Bar g;
+
+struct derp {
+  // Can't be used in a constant expression
+  derp(int); // expected-note {{declared here}}
+  consteval operator int() const { return 5; }
+};
+Bar a; // expected-note {{in instantiation of member function 
'issue_55601::Bar::Bar' requested here}}
+
+struct constantDerp {
+  // Can be used in a constant expression.
+  consteval constantDerp(int) {} 
+  consteval operator int() const { return 5; }
+};
+Bar b;
+
+} // namespace issue_55601
+
 namespace default_argument {
 
 // Previously calls of consteval functions in default arguments were rejected.
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19676,7 +19676,8 @@
 
   if (auto *FD = dyn_cast(E->getDecl()))
 if (!isUnevaluatedContext() && !isConstantEvaluated() &&
-FD->isConsteval() && !RebuildingImmediateInvocation)
+FD->isConsteval() && !RebuildingImmediateInvocation &&
+!FD->isDependentContext())
   ExprEvalContexts.back().ReferenceToConsteval.insert(E);
   MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse,
  RefsMinusAssignments);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -159,6 +159,8 @@
   template instantiation to be constexpr/consteval even though a call to such
   a function cannot appear in a constant expression.
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
+- Correctly defer dependent immediate function invocations until template 
instantiation.
+  This fixes `GH55601 `_.
 
 
 


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -648,9 +648,40 @@
 
 static_assert(bar<15>() == 15);
 static_assert(baz() == sizeof(int));
-
 } // namespace value_dependent
 
+// https://github.com/llvm/llvm-project/issues/55601
+namespace issue_55601 {
+template
+class Bar {
+  consteval static T x() { return 5; }  // expected-note {{non-constexpr constructor 'derp' cannot be used in a constant expression}}
+ public:
+  Bar() : a(x()) {} // expected-error {{call to consteval function 'issue_55601::Bar::x' is not a constant expression}}
+// expected-error@-1 {{call to consteval function 'issue_55601::derp::operator int' is not a constant expression}}
+// expected-note@-2 {{in call to 'x()'}}
+// expected-note@-3 {{non-literal type 'issue_55601::derp' cannot be used in a constant expression}}
+ private:
+  int a;
+};
+Bar f;
+Bar g;
+
+struct derp {
+  // Can't be used in a constant expression
+  derp(int); // expected-note {{declared here}}
+  consteval operator int() const { return 5; }
+};
+Bar a; // expected-note {{in instantiation of member function 'issue_55601::Bar::Bar' requested here}}
+
+struct constantDerp {
+  // Can be used in a constant expression.
+  consteval constantDerp(int) {} 
+  consteval operator int() const { return 5; }
+};
+Bar b;
+
+} // namespace issue_55601
+
 namespace default_argument {
 
 // Previously calls of consteval functions in default arguments were rejected.
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clan

[clang] 0e0e8b6 - Do not evaluate dependent immediate invocations

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2022-08-18T10:30:40+02:00
New Revision: 0e0e8b65765e32776a5188e96d1672baeb11b16c

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

LOG: Do not evaluate dependent immediate invocations

We deferred the evaluation of dependent immediate invocations in 
https://reviews.llvm.org/D119375 until instantiation.
We should also not consider them referenced from a non-consteval context.

Fixes: https://github.com/llvm/llvm-project/issues/55601

```
template
class Bar {
  consteval static T x() { return 5; }
 public:
  Bar() : a(x()) {}

 private:
  int a;
};

Bar g();
```
Is now accepted by clang. Previously it errored with: `cannot take address of 
consteval function 'x' outside of an immediate invocation  Bar() : a(x()) {}`

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/cxx2a-consteval.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 4a6c9deb0c9c2..f74af34abea99 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -159,6 +159,8 @@ C++20 Feature Support
   template instantiation to be constexpr/consteval even though a call to such
   a function cannot appear in a constant expression.
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
+- Correctly defer dependent immediate function invocations until template 
instantiation.
+  This fixes `GH55601 `_.
 
 
 

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 48a00f9de671c..377bfaa5f177d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19676,7 +19676,8 @@ void Sema::MarkDeclRefReferenced(DeclRefExpr *E, const 
Expr *Base) {
 
   if (auto *FD = dyn_cast(E->getDecl()))
 if (!isUnevaluatedContext() && !isConstantEvaluated() &&
-FD->isConsteval() && !RebuildingImmediateInvocation)
+FD->isConsteval() && !RebuildingImmediateInvocation &&
+!FD->isDependentContext())
   ExprEvalContexts.back().ReferenceToConsteval.insert(E);
   MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse,
  RefsMinusAssignments);

diff  --git a/clang/test/SemaCXX/cxx2a-consteval.cpp 
b/clang/test/SemaCXX/cxx2a-consteval.cpp
index 4251d82c17e60..78011e2003417 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -648,9 +648,40 @@ template  constexpr int baz() {
 
 static_assert(bar<15>() == 15);
 static_assert(baz() == sizeof(int));
-
 } // namespace value_dependent
 
+// https://github.com/llvm/llvm-project/issues/55601
+namespace issue_55601 {
+template
+class Bar {
+  consteval static T x() { return 5; }  // expected-note {{non-constexpr 
constructor 'derp' cannot be used in a constant expression}}
+ public:
+  Bar() : a(x()) {} // expected-error {{call to consteval function 
'issue_55601::Bar::x' is not a constant expression}}
+// expected-error@-1 {{call to consteval function 
'issue_55601::derp::operator int' is not a constant expression}}
+// expected-note@-2 {{in call to 'x()'}}
+// expected-note@-3 {{non-literal type 'issue_55601::derp' 
cannot be used in a constant expression}}
+ private:
+  int a;
+};
+Bar f;
+Bar g;
+
+struct derp {
+  // Can't be used in a constant expression
+  derp(int); // expected-note {{declared here}}
+  consteval operator int() const { return 5; }
+};
+Bar a; // expected-note {{in instantiation of member function 
'issue_55601::Bar::Bar' requested here}}
+
+struct constantDerp {
+  // Can be used in a constant expression.
+  consteval constantDerp(int) {} 
+  consteval operator int() const { return 5; }
+};
+Bar b;
+
+} // namespace issue_55601
+
 namespace default_argument {
 
 // Previously calls of consteval functions in default arguments were rejected.



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

[PATCH] D132031: Do not evaluate dependent immediate invocations

[Utkarsh Saxena 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 rG0e0e8b65765e: Do not evaluate dependent immediate 
invocations (authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132031

Files:
  clang/docs/ReleaseNotes.rst
  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
@@ -648,9 +648,40 @@
 
 static_assert(bar<15>() == 15);
 static_assert(baz() == sizeof(int));
-
 } // namespace value_dependent
 
+// https://github.com/llvm/llvm-project/issues/55601
+namespace issue_55601 {
+template
+class Bar {
+  consteval static T x() { return 5; }  // expected-note {{non-constexpr 
constructor 'derp' cannot be used in a constant expression}}
+ public:
+  Bar() : a(x()) {} // expected-error {{call to consteval function 
'issue_55601::Bar::x' is not a constant expression}}
+// expected-error@-1 {{call to consteval function 
'issue_55601::derp::operator int' is not a constant expression}}
+// expected-note@-2 {{in call to 'x()'}}
+// expected-note@-3 {{non-literal type 'issue_55601::derp' 
cannot be used in a constant expression}}
+ private:
+  int a;
+};
+Bar f;
+Bar g;
+
+struct derp {
+  // Can't be used in a constant expression
+  derp(int); // expected-note {{declared here}}
+  consteval operator int() const { return 5; }
+};
+Bar a; // expected-note {{in instantiation of member function 
'issue_55601::Bar::Bar' requested here}}
+
+struct constantDerp {
+  // Can be used in a constant expression.
+  consteval constantDerp(int) {} 
+  consteval operator int() const { return 5; }
+};
+Bar b;
+
+} // namespace issue_55601
+
 namespace default_argument {
 
 // Previously calls of consteval functions in default arguments were rejected.
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -19676,7 +19676,8 @@
 
   if (auto *FD = dyn_cast(E->getDecl()))
 if (!isUnevaluatedContext() && !isConstantEvaluated() &&
-FD->isConsteval() && !RebuildingImmediateInvocation)
+FD->isConsteval() && !RebuildingImmediateInvocation &&
+!FD->isDependentContext())
   ExprEvalContexts.back().ReferenceToConsteval.insert(E);
   MarkExprReferenced(*this, E->getLocation(), E->getDecl(), E, OdrUse,
  RefsMinusAssignments);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -159,6 +159,8 @@
   template instantiation to be constexpr/consteval even though a call to such
   a function cannot appear in a constant expression.
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
+- Correctly defer dependent immediate function invocations until template 
instantiation.
+  This fixes `GH55601 `_.
 
 
 


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -648,9 +648,40 @@
 
 static_assert(bar<15>() == 15);
 static_assert(baz() == sizeof(int));
-
 } // namespace value_dependent
 
+// https://github.com/llvm/llvm-project/issues/55601
+namespace issue_55601 {
+template
+class Bar {
+  consteval static T x() { return 5; }  // expected-note {{non-constexpr constructor 'derp' cannot be used in a constant expression}}
+ public:
+  Bar() : a(x()) {} // expected-error {{call to consteval function 'issue_55601::Bar::x' is not a constant expression}}
+// expected-error@-1 {{call to consteval function 'issue_55601::derp::operator int' is not a constant expression}}
+// expected-note@-2 {{in call to 'x()'}}
+// expected-note@-3 {{non-literal type 'issue_55601::derp' cannot be used in a constant expression}}
+ private:
+  int a;
+};
+Bar f;
+Bar g;
+
+struct derp {
+  // Can't be used in a constant expression
+  derp(int); // expected-note {{declared here}}
+  consteval operator int() const { return 5; }
+};
+Bar a; // expected-note {{in instantiation of member function 'issue_55601::Bar::Bar' requested here}}
+
+struct constantDerp {
+  // Can be used in a constant expression.
+  consteval constantDerp(int) {} 
+  consteval operator int() const { return 5; }
+};
+Bar b;
+
+} // namespace issue_55601
+
 namespace default_argument {
 
 // Previously calls of consteval functions in default arguments were rejected.
Index: clang/lib/Sema/SemaExpr.cpp
===

[PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne added a comment.

In D130586#3731021 , @Ericson2314 
wrote:

> Anyone have any idea what this Debian test failure is about?

I haven’t seen a passing debian pre-merge check for months now, so I usually 
ignore it (the failures seems to be related to debuginfod currently).
The bazel failure seems to be a bug or flake in the build system as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130586

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

[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

[Ties Stuij via Phabricator via cfe-commits]

stuij updated this revision to Diff 453579.
stuij added a comment.

addressed review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

Files:
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/ConstantEmitter.h
  clang/test/CodeGen/const-init.c


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s 
| FileCheck %s
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion 
-ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +183,10 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 
-12312731, i16 -312 }, align 4
 }
+
+// Clang should evaluate this in constant context, so floating point mode 
should
+// have no effect.
+// CHECK: @.compoundliteral = internal global [1 x float] [float 
0x3FB9A000], align 4
+struct { const float *floats; } compoundliteral = {
+  (float[1]) { 0.1, },
+};
Index: clang/lib/CodeGen/ConstantEmitter.h
===
--- clang/lib/CodeGen/ConstantEmitter.h
+++ clang/lib/CodeGen/ConstantEmitter.h
@@ -67,6 +67,9 @@
 return Abstract;
   }
 
+  bool isInConstantContext() const { return InConstantContext; }
+  void setInConstantContext(bool var) { InConstantContext = var; }
+
   /// Try to emit the initiaizer of the given declaration as an abstract
   /// constant.  If this succeeds, the emission must be finalized.
   llvm::Constant *tryEmitForInitializer(const VarDecl &D);
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -913,17 +913,16 @@
 // ConstExprEmitter
 
//===--===//
 
-static ConstantAddress tryEmitGlobalCompoundLiteral(CodeGenModule &CGM,
-CodeGenFunction *CGF,
-  const CompoundLiteralExpr *E) {
+static ConstantAddress
+tryEmitGlobalCompoundLiteral(ConstantEmitter &emitter,
+ const CompoundLiteralExpr *E) {
+  CodeGenModule &CGM = emitter.CGM;
   CharUnits Align = CGM.getContext().getTypeAlignInChars(E->getType());
   if (llvm::GlobalVariable *Addr =
   CGM.getAddrOfConstantCompoundLiteralIfEmitted(E))
 return ConstantAddress(Addr, Addr->getValueType(), Align);
 
   LangAS addressSpace = E->getType().getAddressSpace();
-
-  ConstantEmitter emitter(CGM, CGF);
   llvm::Constant *C = emitter.tryEmitForInitializer(E->getInitializer(),
 addressSpace, 
E->getType());
   if (!C) {
@@ -1967,7 +1966,9 @@
 
 ConstantLValue
 ConstantLValueEmitter::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
-  return tryEmitGlobalCompoundLiteral(CGM, Emitter.CGF, E);
+  ConstantEmitter CompoundLiteralEmitter(CGM, Emitter.CGF);
+  CompoundLiteralEmitter.setInConstantContext(Emitter.isInConstantContext());
+  return tryEmitGlobalCompoundLiteral(CompoundLiteralEmitter, E);
 }
 
 ConstantLValue
@@ -2211,7 +2212,8 @@
 ConstantAddress
 CodeGenModule::GetAddrOfConstantCompoundLiteral(const CompoundLiteralExpr *E) {
   assert(E->isFileScope() && "not a file-scope compound literal expr");
-  return tryEmitGlobalCompoundLiteral(*this, nullptr, E);
+  ConstantEmitter emitter(*this);
+  return tryEmitGlobalCompoundLiteral(emitter, E);
 }
 
 llvm::Constant *


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s | FileCheck %s
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +183,10 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 -12312731, i16 -312 }, align 4
 }
+
+// Clang should evaluate this i

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:55
+  bool VisitMemberExpr(MemberExpr *E) {
+auto *MD = E->getMemberDecl();
+report(E->getMemberLoc(), MD);

nit: inline?



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:55
+  bool VisitMemberExpr(MemberExpr *E) {
+auto *MD = E->getMemberDecl();
+report(E->getMemberLoc(), MD);

sammccall wrote:
> nit: inline?
should this be the founddecl instead of the memberdecl?



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:66
+  bool VisitOverloadExpr(OverloadExpr *E) {
+// Mark all candidates as used when overload is not resolved.
+llvm::for_each(E->decls(),

I think we need a policy knob for this, to decide whether to over or 
underestimate.
This would be the wrong behavior for missing-includes analysis.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:66
+  bool VisitOverloadExpr(OverloadExpr *E) {
+// Mark all candidates as used when overload is not resolved.
+llvm::for_each(E->decls(),

sammccall wrote:
> I think we need a policy knob for this, to decide whether to over or 
> underestimate.
> This would be the wrong behavior for missing-includes analysis.
comment echoes the code, say why instead



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:72
+
+  bool VisitUsingDecl(UsingDecl *UD) {
+for (const auto *Shadow : UD->shadows())

I wonder if this is correct enough.

This brings a set of overloads into scope, the intention may be to bring a 
particular overload with others as harmless side effects: consider `using 
std::swap`.
In this case, inserting includes for all the overloads that happened to be 
visible would be too much.

Possible behaviors:
 - what we do here
 - only do this if overestimate=true
 - if overestimate=false, only include those USDs marked as `referenced` (not 
sure if they actually get marked appropriately, the bit exists)



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:79
+  bool VisitFunctionDecl(FunctionDecl *FD) {
+// Only mark the declaration from a definition.
+if (FD->isThisDeclarationADefinition())

comment echoes the code, say why instead



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:110
+  bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
+// FIXME: Handle specializations.
+return handleTemplateName(TL.getTemplateNameLoc(),

nit: *explicit* specializations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132110

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

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added a comment.
Herald added a project: All.

Hi, I have been trying to get consteval in a better shape.

In D119651#3327212 , @Izaron wrote:

> After an investigation in GDB I can say that the assert seems to be wrong. 
> Since Clang instantiates classes and functions "on the fly" where 
> appropriate, we indeed can get a run-time evaluation context after a 
> compile-time evaluation context. I was sure that evaluation contexts were 
> made to represent a clean hierarchy of context, because they're interrelated, 
> but the case with instantiations confuses me.

Can you rebase and verify if there are still assertion failures. I think the 
assertion is fair. Dependent immediate function invocations must be done during 
instantiation. I suspect https://reviews.llvm.org/D132031 might have fixed the 
issue.

> This code ...
>
>   template
>   struct good_struct {
>   // we are in run-time eval context!
>   static consteval int evalconst() {
>   // we are in compile-time eval context!
>   return N * N;
>   }
>   
>   void foo();
>   void bar();
>   };
>   
>   //int good_struct_100 = good_struct<100>::evalconst();
>   //int good_struct_200 = good_struct<200>::evalconst();
>   
>   consteval int consteval_foo() {
>   // we are in compile-time eval context!
>   return good_struct<100>::evalconst();
>   }
>   
>   template good_struct<200>::evalconst()>
>   constexpr int templated_foo() {
>   return N;
>   }
>
> ... hits the assert two times, unless we uncomment the lines with 
> `good_struct_100` and `good_struct_200`. That's because Clang instantiates 
> the classes when it "sees" them, straight from consteval/template contexts. I 
> couldn't come up with a correct code that breaks though.
>
> I am now less sure if the patch (without the assert) is acceptable, what if 
> the concept of "evaluation contexts" needs a revision?..

Can you also give a reproducer for which you see the assertion failure if you 
still see this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119651

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

[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

[Ties Stuij via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/const-init.c:2
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion 
-ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s

DavidSpickett wrote:
> Drive by comment, what is "the right thing" here? Without me having to git 
> blame this file if I'm looking at this months from now.
> 
> You could add a comment down next to the test like "clang should evaluate 
> this in a constant context, so floating point mode should have no effect". 
> Which is still a bit vague but better than "clang should not crash if we do 
> this".
> 
> Also are there any other floating point modes that have/had this same 
> problem? They should be tested too if so.
Yea, that does make the intent clearer. Added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

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

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Sam McCall via Phabricator via cfe-commits]

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

Yeah, this seems reasonable to me.
Wonder if we can support this in hover too!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

[David Spickett via Phabricator via cfe-commits]

DavidSpickett accepted this revision.
DavidSpickett added a comment.

My comment was addressed, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

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

[PATCH] D130688: [Driver][Sparc] Default to -mcpu=v9 for SparcV8 on Linux

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

It's almost three weeks since the last comments.  Any suggestions on how to 
proceed with this patch?  Given that the original issue (atomics not inlined 
with `-m32`) is now worked around by always linking with `--as-needed -latomic 
--no-as-needed`, the patch is no longer needed to fix build failures.  However, 
it still would make a nice and cheap optimization on distros that require v9 
CPUs, if we can figure out how to reliably identify them.  Maybe someone from 
the LEON community could chime in?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130688

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

[PATCH] D101156: [Clang] Support a user-defined __dso_handle

[Jan Köster via Phabricator via cfe-commits]

Tuxist added a comment.
Herald added a project: All.

i have a problem i'am implement a libc in c++ i can't export now __dso_handle 
as weak so I can't link other shared libs against my libc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101156

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

[PATCH] D130688: [Driver][Sparc] Default to -mcpu=v9 for SparcV8 on Linux

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

In D130688#3731577 , @ro wrote:

> It's almost three weeks since the last comments.  Any suggestions on how to 
> proceed with this patch?  Given that the original issue (atomics not inlined 
> with `-m32`) is now worked around by always linking with `--as-needed 
> -latomic --no-as-needed`, the patch is no longer needed to fix build 
> failures.  However, it still would make a nice and cheap optimization on 
> distros that require v9 CPUs, if we can figure out how to reliably identify 
> them.  Maybe someone from the LEON community could chime in?

Yeah, someone from the LEON community should comment whether they would be OK 
to default to V9 on all Linux targets. I don't really want to omit Gentoo here 
which still support Linux on SPARC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130688

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

[PATCH] D130688: [Driver][Sparc] Default to -mcpu=v9 for SparcV8 on Linux

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D130688#3731611 , @glaubitz wrote:

> 



> Yeah, someone from the LEON community should comment whether they would be OK 
> to default to V9 on all Linux targets. I don't really want to omit Gentoo 
> here which still support Linux on SPARC.

How could they?  LEON is V8 (with extensions) only!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130688

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

[PATCH] D130688: [Driver][Sparc] Default to -mcpu=v9 for SparcV8 on Linux

[John Paul Adrian Glaubitz via Phabricator via cfe-commits]

glaubitz added a comment.

In D130688#3731619 , @ro wrote:

> In D130688#3731611 , @glaubitz 
> wrote:
>
>> 
>
>
>
>> Yeah, someone from the LEON community should comment whether they would be 
>> OK to default to V9 on all Linux targets. I don't really want to omit Gentoo 
>> here which still support Linux on SPARC.
>
> How could they?  LEON is V8 (with extensions) only!

Well, they could patch LLVM locally. I assume they don't use the pristine 
upstream sources anyway in such custom setups.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130688

___
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]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:649
+  return DiscardResult ? this->emitPop(T, E) : true;
+});
   case UO_Not:// ~x

Handling the deref like this breaks assigning, e.g. `int m = 10; int *p = &m; 
*p = 12;` doesn't work. The LHS of the assignment ends up being `*p`, so just 
an int.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132111

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

[clang] 27cbfa7 - [Clang] Propagate const context info when emitting compound literal

[Ties Stuij via cfe-commits]

Author: Ties Stuij
Date: 2022-08-18T11:25:20+01:00
New Revision: 27cbfa7cc8cdab121842adf4dd31f6811f523928

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

LOG: [Clang] Propagate const context info when emitting compound literal

This patch fixes a crash when trying to emit a constant compound literal.

For C++ Clang evaluates either casts or binary operations at translation time,
but doesn't pass on the InConstantContext information that was inferred when
parsing the statement.  Because of this, strict FP evaluation (-ftrapping-math)
which shouldn't be in effect yet, then causes checkFloatingpointResult to return
false, which in tryEmitGlobalCompoundLiteral will trigger an assert that the
compound literal wasn't constant.

The discussion here around 'manifestly constant evaluated contexts' was very
helpful to me when trying to understand what LLVM's position is on what
evaluation context should be in effect, together with the explanatory text in
that patch itself:
https://reviews.llvm.org/D87528

Reviewed By: rjmccall, DavidSpickett

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

Added: 


Modified: 
clang/lib/CodeGen/CGExprConstant.cpp
clang/lib/CodeGen/ConstantEmitter.h
clang/test/CodeGen/const-init.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGExprConstant.cpp 
b/clang/lib/CodeGen/CGExprConstant.cpp
index f00ada98aa55b..db6341e87933a 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -913,17 +913,16 @@ bool ConstStructBuilder::UpdateStruct(ConstantEmitter 
&Emitter,
 // ConstExprEmitter
 
//===--===//
 
-static ConstantAddress tryEmitGlobalCompoundLiteral(CodeGenModule &CGM,
-CodeGenFunction *CGF,
-  const CompoundLiteralExpr *E) {
+static ConstantAddress
+tryEmitGlobalCompoundLiteral(ConstantEmitter &emitter,
+ const CompoundLiteralExpr *E) {
+  CodeGenModule &CGM = emitter.CGM;
   CharUnits Align = CGM.getContext().getTypeAlignInChars(E->getType());
   if (llvm::GlobalVariable *Addr =
   CGM.getAddrOfConstantCompoundLiteralIfEmitted(E))
 return ConstantAddress(Addr, Addr->getValueType(), Align);
 
   LangAS addressSpace = E->getType().getAddressSpace();
-
-  ConstantEmitter emitter(CGM, CGF);
   llvm::Constant *C = emitter.tryEmitForInitializer(E->getInitializer(),
 addressSpace, 
E->getType());
   if (!C) {
@@ -1967,7 +1966,9 @@ ConstantLValueEmitter::VisitConstantExpr(const 
ConstantExpr *E) {
 
 ConstantLValue
 ConstantLValueEmitter::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
-  return tryEmitGlobalCompoundLiteral(CGM, Emitter.CGF, E);
+  ConstantEmitter CompoundLiteralEmitter(CGM, Emitter.CGF);
+  CompoundLiteralEmitter.setInConstantContext(Emitter.isInConstantContext());
+  return tryEmitGlobalCompoundLiteral(CompoundLiteralEmitter, E);
 }
 
 ConstantLValue
@@ -2211,7 +2212,8 @@ void CodeGenModule::setAddrOfConstantCompoundLiteral(
 ConstantAddress
 CodeGenModule::GetAddrOfConstantCompoundLiteral(const CompoundLiteralExpr *E) {
   assert(E->isFileScope() && "not a file-scope compound literal expr");
-  return tryEmitGlobalCompoundLiteral(*this, nullptr, E);
+  ConstantEmitter emitter(*this);
+  return tryEmitGlobalCompoundLiteral(emitter, E);
 }
 
 llvm::Constant *

diff  --git a/clang/lib/CodeGen/ConstantEmitter.h 
b/clang/lib/CodeGen/ConstantEmitter.h
index 188b82e56f536..1a7a181ca7f03 100644
--- a/clang/lib/CodeGen/ConstantEmitter.h
+++ b/clang/lib/CodeGen/ConstantEmitter.h
@@ -67,6 +67,9 @@ class ConstantEmitter {
 return Abstract;
   }
 
+  bool isInConstantContext() const { return InConstantContext; }
+  void setInConstantContext(bool var) { InConstantContext = var; }
+
   /// Try to emit the initiaizer of the given declaration as an abstract
   /// constant.  If this succeeds, the emission must be finalized.
   llvm::Constant *tryEmitForInitializer(const VarDecl &D);

diff  --git a/clang/test/CodeGen/const-init.c b/clang/test/CodeGen/const-init.c
index 551c63e3a4be0..4748d71dca966 100644
--- a/clang/test/CodeGen/const-init.c
+++ b/clang/test/CodeGen/const-init.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s 
| FileCheck %s
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wn

[PATCH] D131555: [Clang] Propagate const context info when emitting compound literal

[Ties Stuij via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
stuij marked an inline comment as done.
Closed by commit rG27cbfa7cc8cd: [Clang] Propagate const context info when 
emitting compound literal (authored by stuij).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131555

Files:
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/ConstantEmitter.h
  clang/test/CodeGen/const-init.c


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s 
| FileCheck %s
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu 
-ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion 
-ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +183,10 @@
 #pragma pack()
   // CHECK: @g31.a = internal global %struct.anon.2 { i16 23122, i32 
-12312731, i16 -312 }, align 4
 }
+
+// Clang should evaluate this in constant context, so floating point mode 
should
+// have no effect.
+// CHECK: @.compoundliteral = internal global [1 x float] [float 
0x3FB9A000], align 4
+struct { const float *floats; } compoundliteral = {
+  (float[1]) { 0.1, },
+};
Index: clang/lib/CodeGen/ConstantEmitter.h
===
--- clang/lib/CodeGen/ConstantEmitter.h
+++ clang/lib/CodeGen/ConstantEmitter.h
@@ -67,6 +67,9 @@
 return Abstract;
   }
 
+  bool isInConstantContext() const { return InConstantContext; }
+  void setInConstantContext(bool var) { InConstantContext = var; }
+
   /// Try to emit the initiaizer of the given declaration as an abstract
   /// constant.  If this succeeds, the emission must be finalized.
   llvm::Constant *tryEmitForInitializer(const VarDecl &D);
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -913,17 +913,16 @@
 // ConstExprEmitter
 
//===--===//
 
-static ConstantAddress tryEmitGlobalCompoundLiteral(CodeGenModule &CGM,
-CodeGenFunction *CGF,
-  const CompoundLiteralExpr *E) {
+static ConstantAddress
+tryEmitGlobalCompoundLiteral(ConstantEmitter &emitter,
+ const CompoundLiteralExpr *E) {
+  CodeGenModule &CGM = emitter.CGM;
   CharUnits Align = CGM.getContext().getTypeAlignInChars(E->getType());
   if (llvm::GlobalVariable *Addr =
   CGM.getAddrOfConstantCompoundLiteralIfEmitted(E))
 return ConstantAddress(Addr, Addr->getValueType(), Align);
 
   LangAS addressSpace = E->getType().getAddressSpace();
-
-  ConstantEmitter emitter(CGM, CGF);
   llvm::Constant *C = emitter.tryEmitForInitializer(E->getInitializer(),
 addressSpace, 
E->getType());
   if (!C) {
@@ -1967,7 +1966,9 @@
 
 ConstantLValue
 ConstantLValueEmitter::VisitCompoundLiteralExpr(const CompoundLiteralExpr *E) {
-  return tryEmitGlobalCompoundLiteral(CGM, Emitter.CGF, E);
+  ConstantEmitter CompoundLiteralEmitter(CGM, Emitter.CGF);
+  CompoundLiteralEmitter.setInConstantContext(Emitter.isInConstantContext());
+  return tryEmitGlobalCompoundLiteral(CompoundLiteralEmitter, E);
 }
 
 ConstantLValue
@@ -2211,7 +2212,8 @@
 ConstantAddress
 CodeGenModule::GetAddrOfConstantCompoundLiteral(const CompoundLiteralExpr *E) {
   assert(E->isFileScope() && "not a file-scope compound literal expr");
-  return tryEmitGlobalCompoundLiteral(*this, nullptr, E);
+  ConstantEmitter emitter(*this);
+  return tryEmitGlobalCompoundLiteral(emitter, E);
 }
 
 llvm::Constant *


Index: clang/test/CodeGen/const-init.c
===
--- clang/test/CodeGen/const-init.c
+++ clang/test/CodeGen/const-init.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -emit-llvm -o - %s | FileCheck %s
+// setting strict FP behaviour in the run line below tests that the compiler
+// does the right thing for global compound literals (compoundliteral test)
+// RUN: %clang_cc1 -no-opaque-pointers -triple i386-pc-linux-gnu -ffreestanding -Wno-pointer-to-int-cast -Wno-int-conversion -ffp-exception-behavior=strict -emit-llvm -o - %s | FileCheck %s
 
 #include 
 
@@ -181,3 +183,10 @@
 #pragma pa

[PATCH] D130688: [Driver][Sparc] Default to -mcpu=v9 for SparcV8 on Linux

[Rainer Orth via Phabricator via cfe-commits]

ro added a comment.

In D130688#3731653 , @glaubitz wrote:

> In D130688#3731619 , @ro wrote:
>
>> In D130688#3731611 , @glaubitz 
>> wrote:
>>
>>> 
>>
>>
>>
>>> Yeah, someone from the LEON community should comment whether they would be 
>>> OK to default to V9 on all Linux targets. I don't really want to omit 
>>> Gentoo here which still support Linux on SPARC.
>>
>> How could they?  LEON is V8 (with extensions) only!
>
> Well, they could patch LLVM locally. I assume they don't use the pristine 
> upstream sources anyway in such custom setups.

Why should they need to?  AFAICS all the LEON CPU support is upstream already, 
so why break upstream for them without need?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130688

___
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]

tbaeder updated this revision to Diff 453610.

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
@@ -32,3 +32,12 @@
 static_assert(!0, "");
 static_assert(-true, "");
 static_assert(-false, ""); //expected-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;
+}
Index: clang/test/AST/Interp/cxx20.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/cxx20.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s -DREFERENCE
+
+
+// expected-no-diagnostics
+constexpr int getMinus5() {
+  int a = 10;
+  a = -5;
+  int *p = &a;
+  return *p;
+}
+
+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
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
@@ -103,6 +103,7 @@
   if (!this->emitLoadPop(T, CE))
 return false;
   return DiscardResult ? this->emitPop(T, CE) : true;
+  return true;
 });
   }
 
@@ -209,6 +210,12 @@
   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;
+  // TODO: Assignments return the assigned value, so the pop() here
+  //   should proably just go away.
+  return this->emitPopPtr(BO);
 default:
   return this->bail(BO);
 }
@@ -596,8 +603,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++
@@ -607,16 +613,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
@@ -628,6 +650,24 @@
   return false;
 }
 
+template 
+bool ByteCodeExprGen::VisitDeclRefExpr(const DeclRefExpr *E) {
+  if (auto It = Locals.find(E->getDecl()); It != Locals.end()) {
+const unsigned Offset = It->second.Offset;
+return this->emitGetPtrLocal(Offset, E);
+  } else if (auto GlobalIndex = P.getGlobal(E->getDecl())) {
+return this->emitGetPtrGlobal(*GlobalIndex, E);
+  } else if (isa(E->getDecl())) {
+// I'm pretty sure we should do something here, BUT
+// when we're in evaluateAsRValue(), we don't have any parameters,
+// so we can't actually use this->Params. This happens when
+// a parameter is used in a return statement.
+return false;
+  }
+
+  return false;
+}
+
 template 
 void ByteCodeExprGen::emitCleanup() {
   for (VariableScope *C = VarScope; C; C = C->getParent())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lis

[PATCH] D131623: [clang-tidy] Improve modernize-use-emplace check

[Joey Watts via Phabricator via cfe-commits]

joeywatts added a comment.

In D131623#3731106 , @njames93 wrote:

> In D131623#3730833 , @joeywatts 
> wrote:
>
>> Thanks for the review @njames93! This is my first contribution so I don't 
>> think I have permission to land this myself, is there someone that can do 
>> that for me?
>
> Sure, which email would you like me to use for the commit?

Please use jwatt...@bloomberg.net!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131623

___
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]

tbaeder updated this revision to Diff 453617.

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
@@ -32,3 +32,12 @@
 static_assert(!0, "");
 static_assert(-true, "");
 static_assert(-false, ""); //expected-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;
+}
Index: clang/test/AST/Interp/cxx20.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/cxx20.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s -DREFERENCE
+
+
+// expected-no-diagnostics
+constexpr int getMinus5() {
+  int a = 10;
+  a = -5;
+  int *p = &a;
+  return *p;
+}
+
+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
@@ -103,6 +103,7 @@
   if (!this->emitLoadPop(T, CE))
 return false;
   return DiscardResult ? this->emitPop(T, CE) : true;
+  return true;
 });
   }
 
@@ -209,6 +210,12 @@
   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;
+  // TODO: Assignments return the assigned value, so the pop() here
+  //   should proably just go away.
+  return this->emitPopPtr(BO);
 default:
   return this->bail(BO);
 }
@@ -596,8 +603,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++
@@ -607,16 +613,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
@@ -628,6 +650,24 @@
   return false;
 }
 
+template 
+bool ByteCodeExprGen::VisitDeclRefExpr(const DeclRefExpr *E) {
+  if (auto It = Locals.find(E->getDecl()); It != Locals.end()) {
+const unsigned Offset = It->second.Offset;
+return this->emitGetPtrLocal(Offset, E);
+  } else if (auto GlobalIndex = P.getGlobal(E->getDecl())) {
+return this->emitGetPtrGlobal(*GlobalIndex, E);
+  } else if (isa(E->getDecl())) {
+// I'm pretty sure we should do something here, BUT
+// when we're in evaluateAsRValue(), we don't have any parameters,
+// so we can't actually use this->Params. This happens when
+// a parameter is used in a return statement.
+return false;
+  }
+
+  return false;
+}
+
 template 
 void ByteCodeExprGen

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

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

tbaeder updated this revision to Diff 453618.

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
@@ -32,3 +32,12 @@
 static_assert(!0, "");
 static_assert(-true, "");
 static_assert(-false, ""); //expected-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;
+}
Index: clang/test/AST/Interp/cxx20.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/cxx20.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s -DREFERENCE
+
+
+// expected-no-diagnostics
+constexpr int getMinus5() {
+  int a = 10;
+  a = -5;
+  int *p = &a;
+  return *p;
+}
+
+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
@@ -209,6 +209,12 @@
   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;
+  // TODO: Assignments return the assigned value, so the pop() here
+  //   should proably just go away.
+  return this->emitPopPtr(BO);
 default:
   return this->bail(BO);
 }
@@ -596,8 +602,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++
@@ -607,16 +612,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
@@ -628,6 +649,24 @@
   return false;
 }
 
+template 
+bool ByteCodeExprGen::VisitDeclRefExpr(const DeclRefExpr *E) {
+  if (auto It = Locals.find(E->getDecl()); It != Locals.end()) {
+const unsigned Offset = It->second.Offset;
+return this->emitGetPtrLocal(Offset, E);
+  } else if (auto GlobalIndex = P.getGlobal(E->getDecl())) {
+return this->emitGetPtrGlobal(*GlobalIndex, E);
+  } else if (isa(E->getDecl())) {
+// I'm pretty sure we should do something here, BUT
+// when we're in evaluateAsRValue(), we don't have any parameters,
+// so we can't actually use this->Params. This happens when
+// a parameter is used in a return statement.
+return false;
+  }
+
+  return false;
+}
+
 template 
 void ByteCodeExprGen::emitCleanup() {
   for (VariableScope *C = VarScope; C; C = C->getParent())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm

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

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

tbaeder updated this revision to Diff 453619.

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
@@ -32,3 +32,12 @@
 static_assert(!0, "");
 static_assert(-true, "");
 static_assert(-false, ""); //expected-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;
+}
Index: clang/test/AST/Interp/cxx20.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/cxx20.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s -DREFERENCE
+
+
+// expected-no-diagnostics
+constexpr int getMinus5() {
+  int a = 10;
+  a = -5;
+  int *p = &a;
+  return *p;
+}
+
+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
@@ -209,6 +209,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);
 }
@@ -596,8 +600,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++
@@ -607,16 +610,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
@@ -628,6 +647,24 @@
   return false;
 }
 
+template 
+bool ByteCodeExprGen::VisitDeclRefExpr(const DeclRefExpr *E) {
+  if (auto It = Locals.find(E->getDecl()); It != Locals.end()) {
+const unsigned Offset = It->second.Offset;
+return this->emitGetPtrLocal(Offset, E);
+  } else if (auto GlobalIndex = P.getGlobal(E->getDecl())) {
+return this->emitGetPtrGlobal(*GlobalIndex, E);
+  } else if (isa(E->getDecl())) {
+// I'm pretty sure we should do something here, BUT
+// when we're in evaluateAsRValue(), we don't have any parameters,
+// so we can't actually use this->Params. This happens when
+// a parameter is used in a return statement.
+return false;
+  }
+
+  return false;
+}
+
 template 
 void ByteCodeExprGen::emitCleanup() {
   for (VariableScope *C = VarScope; C; C = C->getParent())
___
cfe-commits mailing 

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:546
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::Macro);
+return true;

actually, would HighlightingKind::LocalVariable be more appropriate?

The standard calls this a "The function-local predefined variable".

Admittedly I have a tendency to think of it as macro-like by analogy with 
`__LINE__` etc but this model is observably wrong: it breaks down with 
templates and in other ways.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[clang-tools-extra] 9ad0ace - [clang-tidy] Rename a local cmake variables to match the new tool name. NFC.

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2022-08-18T14:27:45+03:00
New Revision: 9ad0ace2ba52b2194090a0ec4dd980d604ea74b0

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

LOG: [clang-tidy] Rename a local cmake variables to match the new tool name. 
NFC.

This shouldn't have any externally visible effect.

This matches the new name from 18b4a8bcf3553174f770f09528c9bd01c8cebfe7.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index de76b4b00c360..cb97858ab7005 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -7,14 +7,14 @@ set(CLANG_TIDY_CONFUSABLE_CHARS_GEN 
"clang-tidy-confusable-chars-gen" CACHE
   STRING "Host clang-tidy-confusable-chars-gen executable. Saves building if 
cross-compiling.")
 
 if(NOT CLANG_TIDY_CONFUSABLE_CHARS_GEN STREQUAL 
"clang-tidy-confusable-chars-gen")
-  set(make_confusable_table ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
-  set(make_confusable_table_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen_target 
${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
 elseif(LLVM_USE_HOST_TOOLS)
-  build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
-  set(make_confusable_table_target "${make_confusable_table}")
+  build_native_tool(clang-tidy-confusable-chars-gen 
clang_tidy_confusable_chars_gen)
+  set(clang_tidy_confusable_chars_gen_target 
"${clang_tidy_confusable_chars_gen}")
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target clang-tidy-confusable-chars-gen)
+  set(clang_tidy_confusable_chars_gen 
$)
+  set(clang_tidy_confusable_chars_gen_target clang-tidy-confusable-chars-gen)
 endif()
 
 add_subdirectory(ConfusableTable)
@@ -22,8 +22,8 @@ add_subdirectory(ConfusableTable)
 
 add_custom_command(
 OUTPUT Confusables.inc
-COMMAND ${make_confusable_table} 
${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt 
${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
-DEPENDS ${make_confusable_table_target} ConfusableTable/confusables.txt)
+COMMAND ${clang_tidy_confusable_chars_gen} 
${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt 
${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
+DEPENDS ${clang_tidy_confusable_chars_gen_target} 
ConfusableTable/confusables.txt)
 
 add_custom_target(genconfusable DEPENDS Confusables.inc)
 



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

[PATCH] D130701: [clang-tidy] Rename a local cmake variables to match the new tool name. NFC.

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ad0ace2ba52: [clang-tidy] Rename a local cmake variables to 
match the new tool name. NFC. (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130701

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt


Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -7,14 +7,14 @@
   STRING "Host clang-tidy-confusable-chars-gen executable. Saves building if 
cross-compiling.")
 
 if(NOT CLANG_TIDY_CONFUSABLE_CHARS_GEN STREQUAL 
"clang-tidy-confusable-chars-gen")
-  set(make_confusable_table ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
-  set(make_confusable_table_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen_target 
${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
 elseif(LLVM_USE_HOST_TOOLS)
-  build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
-  set(make_confusable_table_target "${make_confusable_table}")
+  build_native_tool(clang-tidy-confusable-chars-gen 
clang_tidy_confusable_chars_gen)
+  set(clang_tidy_confusable_chars_gen_target 
"${clang_tidy_confusable_chars_gen}")
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target clang-tidy-confusable-chars-gen)
+  set(clang_tidy_confusable_chars_gen 
$)
+  set(clang_tidy_confusable_chars_gen_target clang-tidy-confusable-chars-gen)
 endif()
 
 add_subdirectory(ConfusableTable)
@@ -22,8 +22,8 @@
 
 add_custom_command(
 OUTPUT Confusables.inc
-COMMAND ${make_confusable_table} 
${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt 
${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
-DEPENDS ${make_confusable_table_target} ConfusableTable/confusables.txt)
+COMMAND ${clang_tidy_confusable_chars_gen} 
${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt 
${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
+DEPENDS ${clang_tidy_confusable_chars_gen_target} 
ConfusableTable/confusables.txt)
 
 add_custom_target(genconfusable DEPENDS Confusables.inc)
 


Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -7,14 +7,14 @@
   STRING "Host clang-tidy-confusable-chars-gen executable. Saves building if cross-compiling.")
 
 if(NOT CLANG_TIDY_CONFUSABLE_CHARS_GEN STREQUAL "clang-tidy-confusable-chars-gen")
-  set(make_confusable_table ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
-  set(make_confusable_table_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
+  set(clang_tidy_confusable_chars_gen_target ${CLANG_TIDY_CONFUSABLE_CHARS_GEN})
 elseif(LLVM_USE_HOST_TOOLS)
-  build_native_tool(clang-tidy-confusable-chars-gen make_confusable_table)
-  set(make_confusable_table_target "${make_confusable_table}")
+  build_native_tool(clang-tidy-confusable-chars-gen clang_tidy_confusable_chars_gen)
+  set(clang_tidy_confusable_chars_gen_target "${clang_tidy_confusable_chars_gen}")
 else()
-  set(make_confusable_table $)
-  set(make_confusable_table_target clang-tidy-confusable-chars-gen)
+  set(clang_tidy_confusable_chars_gen $)
+  set(clang_tidy_confusable_chars_gen_target clang-tidy-confusable-chars-gen)
 endif()
 
 add_subdirectory(ConfusableTable)
@@ -22,8 +22,8 @@
 
 add_custom_command(
 OUTPUT Confusables.inc
-COMMAND ${make_confusable_table} ${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt ${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
-DEPENDS ${make_confusable_table_target} ConfusableTable/confusables.txt)
+COMMAND ${clang_tidy_confusable_chars_gen} ${CMAKE_CURRENT_SOURCE_DIR}/ConfusableTable/confusables.txt ${CMAKE_CURRENT_BINARY_DIR}/Confusables.inc
+DEPENDS ${clang_tidy_confusable_chars_gen_target} ConfusableTable/confusables.txt)
 
 add_custom_target(genconfusable DEPENDS Confusables.inc)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Thanks! It seems like this now finally works as it should again, with the code 
I regularly build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131874

___
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]

tbaeder updated this revision to Diff 453624.

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
@@ -32,3 +32,16 @@
 static_assert(!0, "");
 static_assert(-true, "");
 static_assert(-false, ""); //expected-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;
+}
+
+constexpr int gimme(int k) {
+  return k;
+}
Index: clang/test/AST/Interp/cxx20.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/cxx20.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s -DREFERENCE
+
+
+// expected-no-diagnostics
+constexpr int getMinus5() {
+  int a = 10;
+  a = -5;
+  int *p = &a;
+  return *p;
+}
+
+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
@@ -209,6 +209,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);
 }
@@ -596,8 +600,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++
@@ -607,16 +610,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
@@ -628,6 +647,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);
+  } 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())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ping - is there any interest in this - a single flag for pointing towards 
existing prebuilt native executables instead of having to name every one 
(llvm-tblgen, clang-tblgen, lldb-tblgen, clang-pseudo-gen, 
clang-tidy-confusable-chars-gen) individually?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131052

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

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

FWIW I have no idea: in principle this makes sense, but I don't use such a 
configuration and don't have a clear idea of what people who do use it want.

It also adds significant CMake complexity: e.g. clang-pseudo-gen now has 30 
lines just to support the non-default "native tools" configuration, and this is 
duplicated for each tool. Maybe this could be made cheaper by sharing more of 
this logic in a separate place, but we should probably only add it at all if 
this really is helping someone significantly.

(Lines of CMake logic are extremely expensive to maintain IME: tooling and 
diagnostics are poor, there are no tests, there are too many configurations to 
reason about, interacting components are not documented, the language is 
inherently confusing and many of us that maintain it have only a rudimentary 
understanding)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131052

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

[PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

[John Ericson via Phabricator via cfe-commits]

Ericson2314 added a comment.

OK I was worried the debuginfod one was unique to this PR; glad to hear it 
isn't. I'll give it a shot then, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130586

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

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 453638.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Adress comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132110

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -25,6 +25,7 @@
   Inputs.ExtraFiles["target.h"] = Target.code().str();
   Inputs.ExtraArgs.push_back("-include");
   Inputs.ExtraArgs.push_back("target.h");
+  Inputs.ExtraArgs.push_back("-std=c++17");
   TestAST AST(Inputs);
   const auto &SM = AST.sourceManager();
 
@@ -84,6 +85,8 @@
   testWalk("struct S { static int ^x; };", "int y = S::^x;");
   // Canonical declaration only.
   testWalk("extern int ^x; int x;", "int y = ^x;");
+  // Return type of `foo` isn't used.
+  testWalk("struct S{}; S ^foo();", "auto bar() { return ^foo(); }");
 }
 
 TEST(WalkAST, TagType) {
@@ -98,11 +101,66 @@
 using ns::^x;
   )cpp",
"int y = ^x;");
+  testWalk("using ^foo = int;", "^foo x;");
+  testWalk("struct S {}; using ^foo = S;", "^foo x;");
+}
+
+TEST(WalkAST, Using) {
+  testWalk("namespace ns { void ^x(); void ^x(int); }", "using ns::^x;");
+  testWalk("namespace ns { struct S; } using ns::^S;", "^S *s;");
+}
+
+TEST(WalkAST, Namespaces) {
+  testWalk("namespace ns { void x(); }", "using namespace ^ns;");
+}
+
+TEST(WalkAST, TemplateNames) {
+  testWalk("template struct ^S {};", "^S s;");
+  // FIXME: Template decl has the wrong primary location for type-alias template
+  // decls.
   testWalk(R"cpp(
-namespace ns { struct S; } // Not used
-using ns::S; // FIXME: S should be used
-  )cpp",
-   "^S *x;");
+  template  struct S {};
+  template  ^using foo = S;)cpp",
+   "^foo x;");
+  testWalk(R"cpp(
+  namespace ns {template  struct S {}; }
+  using ns::^S;)cpp",
+   "^S x;");
+  testWalk("template struct ^S {};",
+   R"cpp(
+  template  typename> struct X {};
+  X<^S> x;)cpp");
+  testWalk("template struct ^S { S(T); };", "^S s(42);");
+  // Should we mark the specialization instead?
+  testWalk("template struct ^S {}; template <> struct S {};",
+   "^S s;");
+}
+
+TEST(WalkAST, MemberExprs) {
+  testWalk("struct S { void ^foo(); };", "void foo() { S{}.^foo(); }");
+  testWalk("struct S { void foo(); }; struct X : S { using S::^foo; };",
+   "void foo() { X{}.^foo(); }");
+}
+
+TEST(WalkAST, ConstructExprs) {
+  testWalk("struct ^S {};", "S ^t;");
+  testWalk("struct S { ^S(int); };", "S ^t(42);");
+}
+
+TEST(WalkAST, Functions) {
+  // Definition uses declaration, not the other way around.
+  testWalk("void ^foo();", "void ^foo() {}");
+  testWalk("void foo() {}", "void ^foo();");
+
+  // Unresolved calls marks all the overloads.
+  testWalk("void ^foo(int); void ^foo(char);",
+   "template  void bar() { ^foo(T{}); }");
+}
+
+TEST(WalkAST, Enums) {
+  testWalk("enum E { ^A = 42, B = 43 };", "int e = ^A;");
+  testWalk("enum class ^E : int;", "enum class ^E : int {};");
+  testWalk("enum class E : int {};", "enum class ^E : int ;");
 }
 
 } // namespace
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -7,7 +7,17 @@
 //===--===//
 
 #include "AnalysisInternal.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TemplateName.h"
+#include "clang/AST/Type.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Casting.h"
 
 namespace clang {
 namespace include_cleaner {
@@ -17,6 +27,16 @@
 class ASTWalker : public RecursiveASTVisitor {
   DeclCallback Callback;
 
+  bool handleTemplateName(SourceLocation Loc, TemplateName TN) {
+// For using-templates, only mark the alias.
+if (auto *USD = TN.getAsUsingShadowDecl()) {
+  report(Loc, USD);
+  return true;
+}
+report(Loc, TN.getAsTemplateDecl());
+return true;
+  }
+
   void report(SourceLocation Loc, NamedDecl *ND) {
 if (!ND || Loc.isInvalid())
   return;
@@ -26,15 +46,88 @@
 public:
   ASTWalker(DeclCallback Callback) : Callback(Callback) {}
 
+  bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+report(DRE->getLocation(), DRE->getFoundDecl());
+return true;
+  }
+
+  bool VisitMemberExpr(MemberExpr *E) {
+report(

[PATCH] D132110: [IncludeCleaner] Handle more C++ constructs

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:55
+  bool VisitMemberExpr(MemberExpr *E) {
+auto *MD = E->getMemberDecl();
+report(E->getMemberLoc(), MD);

sammccall wrote:
> sammccall wrote:
> > nit: inline?
> should this be the founddecl instead of the memberdecl?
right, forgot about these. also added a test.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:66
+  bool VisitOverloadExpr(OverloadExpr *E) {
+// Mark all candidates as used when overload is not resolved.
+llvm::for_each(E->decls(),

sammccall wrote:
> sammccall wrote:
> > I think we need a policy knob for this, to decide whether to over or 
> > underestimate.
> > This would be the wrong behavior for missing-includes analysis.
> comment echoes the code, say why instead
i agree that this needs a knob. it's just unclear at which level currently, i 
am putting together a doc to have a better decision here (mostly to post vs pre 
filter).

i'd rather move forward with this version, to prepare grounds for the tidy 
check and clangd usage based on this library, and address these issues in a new 
iteration.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:72
+
+  bool VisitUsingDecl(UsingDecl *UD) {
+for (const auto *Shadow : UD->shadows())

sammccall wrote:
> I wonder if this is correct enough.
> 
> This brings a set of overloads into scope, the intention may be to bring a 
> particular overload with others as harmless side effects: consider `using 
> std::swap`.
> In this case, inserting includes for all the overloads that happened to be 
> visible would be too much.
> 
> Possible behaviors:
>  - what we do here
>  - only do this if overestimate=true
>  - if overestimate=false, only include those USDs marked as `referenced` (not 
> sure if they actually get marked appropriately, the bit exists)
i agree, basically the same things as I mentioned above, we should definitely 
have a way to filter these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132110

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

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:546
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::Macro);
+return true;

sammccall wrote:
> actually, would HighlightingKind::LocalVariable be more appropriate?
> 
> The standard calls this a "The function-local predefined variable".
> 
> Admittedly I have a tendency to think of it as macro-like by analogy with 
> `__LINE__` etc but this model is observably wrong: it breaks down with 
> templates and in other ways.
Hm, yes, I was not aware it was specified explicitly as a local variable. Will 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

___
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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added subscribers: shafik, tahonermann.
erichkeane added a comment.

Just the 1 question for my info, Tests seem pretty complete for what is being 
looked at, but I'm hopeful someone else will take a look at this too.  @shafik 
or @tahonermann want to take a pass so aaron doesn't have to?




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:215
+return false;
+  return DiscardResult ? this->emitPopPtr(BO) : true;
 default:

Can you explain what this is doing for me?


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

https://reviews.llvm.org/D132111

___
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]

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:215
+return false;
+  return DiscardResult ? this->emitPopPtr(BO) : true;
 default:

erichkeane wrote:
> Can you explain what this is doing for me?
The `Store` operation pops the value to store from the stack, but leaves the 
pointer it stores the value to on the stack (it only `peek()`s the pointer). So 
after the `emitStore`, there's still the pointer we stored to on the stack, and 
we need to get rid of it if `DiscardResult` is true (that's the case for a 
simple `i = 5;` on a single line). Otherwise, we leave it no the stack for the 
caller to read from.


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

https://reviews.llvm.org/D132111

___
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

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki created this revision.
Herald added a project: All.
yusuke-kadowaki edited the summary of this revision.
yusuke-kadowaki added reviewers: curdeius, MyDeveloperDay, HazardyKnusperkeks.
yusuke-kadowaki published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch addresses https://github.com/llvm/llvm-project/issues/19756


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132131

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -20042,6 +20042,7 @@
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE_BOOL(AlignTrailingComments);
+  CHECK_PARSE_BOOL(AlignTrailingCommentsIgnoreEmptyLine);
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
@@ -26434,6 +26435,15 @@
"inline bool var = is_integral_v && is_signed_v;");
 }
 
+TEST_F(FormatTest, AlignTrailingCommentsIgnoreEmptyLine) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingCommentsIgnoreEmptyLine = true;
+  verifyFormat("#include \"a.h\"  //\n"
+   "\n"
+   "#include \"aa.h\" //\n",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -927,6 +927,7 @@
   unsigned StartOfSequence = 0;
   bool BreakBeforeNext = false;
   unsigned Newlines = 0;
+  unsigned int NewLineThr = Style.AlignTrailingCommentsIgnoreEmptyLine ? 2 : 1;
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
 if (Changes[i].StartOfBlockComment)
   continue;
@@ -979,7 +980,7 @@
   MinColumn = ChangeMinColumn;
   MaxColumn = ChangeMinColumn;
   StartOfSequence = i;
-} else if (BreakBeforeNext || Newlines > 1 ||
+else if (BreakBeforeNext || Newlines > NewLineThr ||
(ChangeMinColumn > MaxColumn || ChangeMaxColumn < MinColumn) ||
// Break the comment sequence if the previous line did not end
// in a trailing comment.
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -647,6 +647,7 @@
 IO.mapOptional("AlignEscapedNewlines", Style.AlignEscapedNewlines);
 IO.mapOptional("AlignOperands", Style.AlignOperands);
 IO.mapOptional("AlignTrailingComments", Style.AlignTrailingComments);
+IO.mapOptional("AlignTrailingCommentsIgnoreEmptyLine", Style.AlignTrailingCommentsIgnoreEmptyLine);
 IO.mapOptional("AllowAllArgumentsOnNextLine",
Style.AllowAllArgumentsOnNextLine);
 IO.mapOptional("AllowAllParametersOfDeclarationOnNextLine",
@@ -1182,6 +1183,7 @@
   LLVMStyle.AlignArrayOfStructures = FormatStyle::AIAS_None;
   LLVMStyle.AlignOperands = FormatStyle::OAS_Align;
   LLVMStyle.AlignTrailingComments = true;
+  LLVMStyle.AlignTrailingCommentsIgnoreEmptyLine = false;
   LLVMStyle.AlignConsecutiveAssignments = {};
   LLVMStyle.AlignConsecutiveAssignments.Enabled = false;
   LLVMStyle.AlignConsecutiveAssignments.AcrossEmptyLines = false;
@@ -1448,6 +1450,7 @@
 GoogleStyle.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
 GoogleStyle.AlignOperands = FormatStyle::OAS_DontAlign;
 GoogleStyle.AlignTrailingComments = false;
+GoogleStyle.AlignTrailingCommentsIgnoreEmptyLine = false;
 GoogleStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Empty;
 GoogleStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
 GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
@@ -1596,6 +1599,7 @@
   Style.AlignAfterOpenBracket = FormatStyle::BAS_DontAlign;
   Style.AlignOperands = FormatStyle::OAS_DontAlign;
   Style.AlignTrailingComments = false;
+  Style.AlignTrailingCommentsIgnoreEmptyLine = false;
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
   Style.BreakBeforeBinaryOperators = FormatStyle::BOS_All;
   Style.BreakBeforeBraces = FormatStyle::BS_WebKit;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -378,6 +378,8 @@
   /// \version 3.7
   bool AlignTrailingComments;
 
+  bool AlignTrailingCommentsIgnoreEmptyLine;
+
   /// \brief If a function call or braced initializer list doesn't fit on a
   /// line, allow putting all arguments onto the next line, even i

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 453648.
ckandeler added a comment.

Use variable instead of macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -841,6 +841,12 @@
 $Function_deprecated[[Foo]]($Parameter[[x]]); 
 }
   )cpp",
+  // Predefined identifiers
+  R"cpp(
+void $Function_decl[[Foo]]() {
+const char *$LocalVariable_decl_readonly[[s]] = 
$LocalVariable_readonly_static[[__func__]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -542,6 +542,14 @@
 return Base::TraverseConstructorInitializer(Init);
   }
 
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::LocalVariable)
+.addModifier(HighlightingModifier::Static)
+.addModifier(HighlightingModifier::Readonly)
+.addModifier(HighlightingModifier::FunctionScope);
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -841,6 +841,12 @@
 $Function_deprecated[[Foo]]($Parameter[[x]]); 
 }
   )cpp",
+  // Predefined identifiers
+  R"cpp(
+void $Function_decl[[Foo]]() {
+const char *$LocalVariable_decl_readonly[[s]] = $LocalVariable_readonly_static[[__func__]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -542,6 +542,14 @@
 return Base::TraverseConstructorInitializer(Init);
   }
 
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::LocalVariable)
+.addModifier(HighlightingModifier::Static)
+.addModifier(HighlightingModifier::Readonly)
+.addModifier(HighlightingModifier::FunctionScope);
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131547: [Clang][AArch64] Use generic extract/insert vector for svget/svset/svcreate tuples

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Thanks for addressing the comments @CarolineConcatto!

In D131547#3731310 , @dmgreen wrote:

>> Is there a formal requirement that LLVM must remain backward compatible with 
>> older LLVM IR (beyond the target-independent parts)?
>
> We have always done it in the past, and I don't see a good reason to change. 
> This change is essentially for llvm 16, so we are talking about any bitcode 
> between when SVE was added and that release. It is hard to tell how people 
> will use bitcode up to that point and if they will expect it to continue 
> working going forward. I think it's simpler to just add the upgrade code, 
> than to try and argue that it is unneeded. But the upgrade code is really 
> needed in D131548  (and D131687 
> ) where the old intrinsics are being 
> removed.

It seems that the LLVM Developer Policy 

 provides better guidance than that.

> Newer releases can ignore features from older releases, but they cannot 
> miscompile them

Removing the intrinsics but not auto-upgrading them would mean that older IR 
would miscompile (the call to the intrinsic would become an actual function 
call). This suggests there is no freedom of choice here and we must use 
AutoUpgrade.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:9099
+ ArrayRef Ops) {
+
+  assert(TypeFlags.isTupleCreate() && "Expects TypleFlag isTupleCreate");

nit: redundant newline, please remove.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131547

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

[PATCH] D131687: [AArch64] Replace aarch64_sve_ldN intrinsic by aarch64_sve_ldN.sret

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Can you also split this patch in two:

- One for Clang where it will no longer use the legacy ld2/3/4 intrinsics
- One for LLVM where it removes the old intrinsics and AutoUpgrades the old 
intrinsics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131687

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

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(thanks, still LG)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[PATCH] D132135: [clangd] Support hover on __func__ etc (PredefinedExpr)

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ckandeler.
Herald added a subscriber: arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Expose these as variables as that's what the standard calls them (and D131175 
).

To make this work, we also fix a bug in SelectionTree: PredefinedExpr has
an implicit/invisible StringLiteral, and SelectionTree should not traverse
implicit things.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132135

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -527,6 +527,10 @@
 /*error-ok*/
 void func() [[{^]])cpp",
"CompoundStmt"},
+  {R"cpp(
+void func() { [[__^func__]]; }
+)cpp",
+   "PredefinedExpr"},
   };
 
   for (const Case &C : Cases) {
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -139,6 +139,33 @@
  HI.Definition = "int bar";
  HI.Type = "int";
}},
+  // Predefined variable
+  {R"cpp(
+  void foo() {
+[[__f^unc__]];
+  }
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "__func__";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Documentation =
+ "Name of the current function (predefined variable)";
+ HI.Value = "\"foo\"";
+ HI.Type = "const char[4]";
+   }},
+  // Predefined variable (dependent)
+  {R"cpp(
+  template void foo() {
+[[__f^unc__]];
+  }
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "__func__";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Documentation =
+ "Name of the current function (predefined variable)";
+ HI.Type = "const char[]";
+   }},
   // Anon namespace and local scope.
   {R"cpp(
   namespace ns1 { namespace {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -720,6 +720,14 @@
 return Base::TraverseTypeConstraint(C);
   }
 
+  // Override child traversal for certain node types.
+  using RecursiveASTVisitor::getStmtChildren;
+  // PredefinedExpr like __func__ has a StringLiteral child for its value.
+  // It's not written, so don't traverse it.
+  Stmt::child_range getStmtChildren(PredefinedExpr *) {
+return {StmtIterator{}, StmtIterator{}};
+  }
+
 private:
   using Base = RecursiveASTVisitor;
 
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -32,6 +32,7 @@
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TokenKinds.h"
@@ -640,6 +641,32 @@
   return HI;
 }
 
+/// The standard defines __func__ as a "predefined variable".
+llvm::Optional
+getPredefinedExprHoverContents(const PredefinedExpr &PE, ASTContext &Ctx,
+   const PrintingPolicy &PP) {
+  HoverInfo HI;
+  HI.Name = PE.getIdentKindName();
+  HI.Kind = index::SymbolKind::Variable;
+  HI.Documentation = "Name of the current function (predefined variable)";
+  if (const StringLiteral *Name = PE.getFunctionName()) {
+HI.Value.emplace();
+llvm::raw_string_ostream OS(*HI.Value);
+Name->outputString(OS);
+HI.Type = printType(Name->getType(), Ctx, PP);
+  } else {
+// Inside templates, the approximate type `const char[]` is still useful.
+QualType CharType = Ctx.CharTy;
+if (Ctx.getLangOpts().CPlusPlus)
+  CharType = CharType.withConst();
+QualType StringType = Ctx.getIncompleteArrayType(
+CharType, ArrayType::ArraySizeModifier::Normal,
+/*IndexTypeQuals=*/0);
+HI.Type = printType(StringType, Ctx, PP);
+  }
+  return HI;
+}
+
 /// Generate a \p Hover object given the macro \p MacroDecl.
 HoverInfo getHoverContents(const DefinedMacro &Macro, ParsedAST &AST) {
   HoverInfo HI;
@@ -764,6 +791,8 @@
   // For `this` expr we currently generate hover with pointee type.
   if (const C

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler marked an inline comment as not done.
ckandeler added a comment.

Thanks for the review. Can you please merge it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[PATCH] D132136: [clang] Perform implicit lvalue-to-rvalue cast with new interpreter

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

tbaeder created this revision.
tbaeder added reviewers: erichkeane, aaron.ballman, shafik, tahonermann.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Extra bonus patch.

The EvaluateAsRValue() documentation mentions that an implicit
lvalue-to-rvalue cast is being performed if the result is an lvalue.
However, that was not being done if the new constant interpreter was in
use.

Just always do it.

This touches `ExprConstant.cpp`, but is a NFC patch if the new constant 
interpreter is not in use.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132136

Files:
  clang/lib/AST/ExprConstant.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
@@ -9,10 +9,8 @@
 
 constexpr int number = 10;
 static_assert(number == 10, "");
-static_assert(number != 10, ""); // expected-error{{failed}}
-#ifdef REFERENCE
-// expected-note@-2{{evaluates to}}
-#endif
+static_assert(number != 10, ""); // expected-error{{failed}} \
+ // expected-note{{evaluates to}}
 
 constexpr bool getTrue() { return true; }
 constexpr bool getFalse() { return false; }
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -14934,25 +14934,27 @@
 /// lvalue-to-rvalue cast if it is an lvalue.
 static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result) {
   assert(!E->isValueDependent());
+
+  if (E->getType().isNull())
+return false;
+
+  if (!CheckLiteralType(Info, E))
+return false;
+
   if (Info.EnableNewConstInterp) {
 if (!Info.Ctx.getInterpContext().evaluateAsRValue(Info, E, Result))
   return false;
   } else {
-if (E->getType().isNull())
-  return false;
-
-if (!CheckLiteralType(Info, E))
-  return false;
-
 if (!::Evaluate(Result, Info, E))
   return false;
+  }
 
-if (E->isGLValue()) {
-  LValue LV;
-  LV.setFrom(Info.Ctx, Result);
-  if (!handleLValueToRValueConversion(Info, E, E->getType(), LV, Result))
-return false;
-}
+  // Implicit lvalue-to-rvalue cast.
+  if (E->isGLValue()) {
+LValue LV;
+LV.setFrom(Info.Ctx, Result);
+if (!handleLValueToRValueConversion(Info, E, E->getType(), LV, Result))
+  return false;
   }
 
   // Check this core constant expression is a constant expression.


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -9,10 +9,8 @@
 
 constexpr int number = 10;
 static_assert(number == 10, "");
-static_assert(number != 10, ""); // expected-error{{failed}}
-#ifdef REFERENCE
-// expected-note@-2{{evaluates to}}
-#endif
+static_assert(number != 10, ""); // expected-error{{failed}} \
+ // expected-note{{evaluates to}}
 
 constexpr bool getTrue() { return true; }
 constexpr bool getFalse() { return false; }
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -14934,25 +14934,27 @@
 /// lvalue-to-rvalue cast if it is an lvalue.
 static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result) {
   assert(!E->isValueDependent());
+
+  if (E->getType().isNull())
+return false;
+
+  if (!CheckLiteralType(Info, E))
+return false;
+
   if (Info.EnableNewConstInterp) {
 if (!Info.Ctx.getInterpContext().evaluateAsRValue(Info, E, Result))
   return false;
   } else {
-if (E->getType().isNull())
-  return false;
-
-if (!CheckLiteralType(Info, E))
-  return false;
-
 if (!::Evaluate(Result, Info, E))
   return false;
+  }
 
-if (E->isGLValue()) {
-  LValue LV;
-  LV.setFrom(Info.Ctx, Result);
-  if (!handleLValueToRValueConversion(Info, E, E->getType(), LV, Result))
-return false;
-}
+  // Implicit lvalue-to-rvalue cast.
+  if (E->isGLValue()) {
+LValue LV;
+LV.setFrom(Info.Ctx, Result);
+if (!handleLValueToRValueConversion(Info, E, E->getType(), LV, Result))
+  return false;
   }
 
   // Check this core constant expression is a constant expression.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132135: [clangd] Support hover on __func__ etc (PredefinedExpr)

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 453652.
sammccall added a comment.

fix type to be const in both c & c++, not sure where I got that idea from...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132135

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/SelectionTests.cpp

Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -527,6 +527,10 @@
 /*error-ok*/
 void func() [[{^]])cpp",
"CompoundStmt"},
+  {R"cpp(
+void func() { [[__^func__]]; }
+)cpp",
+   "PredefinedExpr"},
   };
 
   for (const Case &C : Cases) {
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -139,6 +139,33 @@
  HI.Definition = "int bar";
  HI.Type = "int";
}},
+  // Predefined variable
+  {R"cpp(
+  void foo() {
+[[__f^unc__]];
+  }
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "__func__";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Documentation =
+ "Name of the current function (predefined variable)";
+ HI.Value = "\"foo\"";
+ HI.Type = "const char[4]";
+   }},
+  // Predefined variable (dependent)
+  {R"cpp(
+  template void foo() {
+[[__f^unc__]];
+  }
+  )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "__func__";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Documentation =
+ "Name of the current function (predefined variable)";
+ HI.Type = "const char[]";
+   }},
   // Anon namespace and local scope.
   {R"cpp(
   namespace ns1 { namespace {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -720,6 +720,14 @@
 return Base::TraverseTypeConstraint(C);
   }
 
+  // Override child traversal for certain node types.
+  using RecursiveASTVisitor::getStmtChildren;
+  // PredefinedExpr like __func__ has a StringLiteral child for its value.
+  // It's not written, so don't traverse it.
+  Stmt::child_range getStmtChildren(PredefinedExpr *) {
+return {StmtIterator{}, StmtIterator{}};
+  }
+
 private:
   using Base = RecursiveASTVisitor;
 
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -32,6 +32,7 @@
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TokenKinds.h"
@@ -640,6 +641,29 @@
   return HI;
 }
 
+/// The standard defines __func__ as a "predefined variable".
+llvm::Optional
+getPredefinedExprHoverContents(const PredefinedExpr &PE, ASTContext &Ctx,
+   const PrintingPolicy &PP) {
+  HoverInfo HI;
+  HI.Name = PE.getIdentKindName();
+  HI.Kind = index::SymbolKind::Variable;
+  HI.Documentation = "Name of the current function (predefined variable)";
+  if (const StringLiteral *Name = PE.getFunctionName()) {
+HI.Value.emplace();
+llvm::raw_string_ostream OS(*HI.Value);
+Name->outputString(OS);
+HI.Type = printType(Name->getType(), Ctx, PP);
+  } else {
+// Inside templates, the approximate type `const char[]` is still useful.
+QualType StringType = Ctx.getIncompleteArrayType(
+Ctx.CharTy.withConst(), ArrayType::ArraySizeModifier::Normal,
+/*IndexTypeQuals=*/0);
+HI.Type = printType(StringType, Ctx, PP);
+  }
+  return HI;
+}
+
 /// Generate a \p Hover object given the macro \p MacroDecl.
 HoverInfo getHoverContents(const DefinedMacro &Macro, ParsedAST &AST) {
   HoverInfo HI;
@@ -764,6 +788,8 @@
   // For `this` expr we currently generate hover with pointee type.
   if (const CXXThisExpr *CTE = dyn_cast(E))
 return getThisExprHoverContents(CTE, AST.getASTContext(), PP);
+  if (const PredefinedExpr *PE = dyn_cast(E))
+return getPredefinedExprHoverContents(*PE, AST.getASTContext(), PP);
   // For expressions we currently print the type and the value, iff it is
   // evaluatable.
   if (auto Val = printExprValue(E, AST.getASTContext())) {
___
cfe-commits mailing list
cfe-com

[clang-tools-extra] 1c056f8 - [clangd] Use the "macro" semantic token for pre-defined identifiers

[Sam McCall via cfe-commits]

Author: Christian Kandeler
Date: 2022-08-18T16:12:55+02:00
New Revision: 1c056f8df26b542b21b61a538203244215701113

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

LOG: [clangd] Use the "macro" semantic token for pre-defined identifiers

This matches user expectations.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 4a50853455b92..5b6fcf923e3fa 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -542,6 +542,14 @@ class CollectExtraHighlightings
 return Base::TraverseConstructorInitializer(Init);
   }
 
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::LocalVariable)
+.addModifier(HighlightingModifier::Static)
+.addModifier(HighlightingModifier::Readonly)
+.addModifier(HighlightingModifier::FunctionScope);
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...

diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp 
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 128cd620d710e..32f0f58ade2c3 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -841,6 +841,12 @@ sizeof...($TemplateParameter[[Elements]]);
 $Function_deprecated[[Foo]]($Parameter[[x]]); 
 }
   )cpp",
+  // Predefined identifiers
+  R"cpp(
+void $Function_decl[[Foo]]() {
+const char *$LocalVariable_decl_readonly[[s]] = 
$LocalVariable_readonly_static[[__func__]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};



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

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1c056f8df26b: [clangd] Use the "macro" semantic 
token for pre-defined identifiers (authored by ckandeler, committed by 
sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -841,6 +841,12 @@
 $Function_deprecated[[Foo]]($Parameter[[x]]); 
 }
   )cpp",
+  // Predefined identifiers
+  R"cpp(
+void $Function_decl[[Foo]]() {
+const char *$LocalVariable_decl_readonly[[s]] = 
$LocalVariable_readonly_static[[__func__]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -542,6 +542,14 @@
 return Base::TraverseConstructorInitializer(Init);
   }
 
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::LocalVariable)
+.addModifier(HighlightingModifier::Static)
+.addModifier(HighlightingModifier::Readonly)
+.addModifier(HighlightingModifier::FunctionScope);
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -841,6 +841,12 @@
 $Function_deprecated[[Foo]]($Parameter[[x]]); 
 }
   )cpp",
+  // Predefined identifiers
+  R"cpp(
+void $Function_decl[[Foo]]() {
+const char *$LocalVariable_decl_readonly[[s]] = $LocalVariable_readonly_static[[__func__]];
+}
+  )cpp",
   // Explicit template specialization
   R"cpp(
 struct $Class_decl[[Base]]{};
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -542,6 +542,14 @@
 return Base::TraverseConstructorInitializer(Init);
   }
 
+  bool VisitPredefinedExpr(PredefinedExpr *E) {
+H.addToken(E->getLocation(), HighlightingKind::LocalVariable)
+.addModifier(HighlightingModifier::Static)
+.addModifier(HighlightingModifier::Readonly)
+.addModifier(HighlightingModifier::FunctionScope);
+return true;
+  }
+
   bool VisitCallExpr(CallExpr *E) {
 // Highlighting parameters passed by non-const reference does not really
 // make sense for literals...
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sure thing!

Also sent D132135  to support hover.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[PATCH] D131662: [clang] Try to improve diagnostics about uninitialized constexpr variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaFixItUtils.cpp:208-210
+  if (T->isArrayType()) {
+return " = {}";
+  }

tbaeder wrote:
> aaron.ballman wrote:
> > I don't think this is a good change, consider:
> > ```
> > int array[] = {};
> > ```
> > zero-sized arrays are an extension in both C and C++, and the empty 
> > initializer is a GNU extension in C (at least until C2x).
> The code in `SemaInit.cpp` is weird in that it only holds if the fixit isn't 
> empty... so if that change does away, a `constexpr int arr[2];` also doesn't 
> use the new error message :(
Ugh, well that's rather unfortunate. But I'm not certain the improved 
diagnostic is worth the incorrect fix-it unless the array has specified bounds. 
Perhaps another way to address this is to work on the diagnostic behavior to be 
more like:
```
error: definition of constexpr variable with array type needs an explicit 
initializer
constexpr int a[];
  ^
```
Does that seem more plausible?



Comment at: clang/lib/Sema/SemaInit.cpp:8063
 // handled in the Failed() branch above.
-QualType DestType = Entity.getType();
-S.Diag(Kind.getLocation(), DiagID)
-<< DestType << (bool)DestType->getAs()
-<< FixItHint::CreateInsertion(ZeroInitializationFixitLoc,
-  ZeroInitializationFixit);
+if (!DestType->getAs() && VD && VD->isConstexpr()) {
+  // Use a more useful diagnostic for constexpr variables.

tbaeder wrote:
> aaron.ballman wrote:
> > Why the check for a record type?
> For record types, the current error message would be:
> ```
> default initialization of an object of const type 'const S3' without a 
> user-provided default constructor
> ```
> which gives more information than just "must be initialized by a constant 
> expression".
Yeah, that is a better diagnostic, thanks! Too bad the diagnostic logic is 
separated like this though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131662

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

[PATCH] D132056: [HLSL] Restrict to supported targets

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: ABataev, Anastasia.
aaron.ballman added a comment.

I don't have particular concerns FWIW, but I'm not certain if others have 
opinions so I'm holding off on accepting for a bit.




Comment at: clang/test/Driver/hlsl-lang-targets.hlsl:14
+
+// Maybe someday
+// SPIRV: error: HLSL code generation is unsupported for target 
'spirv64-unknown-unknown'

Thoughts on this change?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132056

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

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131084

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

[PATCH] D130510: Missing tautological compare warnings due to unary operators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130510#3729864 , @rtrieu wrote:

> In D130510#3728719 , @aaron.ballman 
> wrote:
>
>> In D130510#3727096 , @rtrieu wrote:
>>
>>> This patch has been moving back and forth between 
>>> `IsIntegerLiteralConstantExpr` and `getIntegerLiteralSubexpressionValue`.  
>>> The first function is preexisting and the second one is a new function.  
>>> The final patch seems to settle on using just 
>>> `getIntegerLiteralSubexpressionValue`.  Can you explain why the existing 
>>> function does not meet your needs?  It wasn't clear from the update 
>>> messages why you went that way.
>>
>> The existing function returns whether the expression is an ICE (sort of), 
>> the replacement function calculates the actual value and returns an optional 
>> APInt so you can perform operations on it directly. That said, a future 
>> refactoring could probably remove `IsIntegerLiteralConstantExpr()` and 
>> replace it with `getIntegerLiteralSubexpressionValue()`, but the only caller 
>> of `IsIntegerLiteralConstantExpr()` doesn't actually need the value at that 
>> point.
>
> In that case, I suggest adding a comment to pointing to the other function.  
> I expected that some day, someone will notice that the calculation for 
> literals is slightly different between the two warnings and we should be 
> helpful to them.  I have no other concerns.

That's a good suggestion, I've added two suggested edits for the comment. 
Thanks for the extra set of eyes on this review @rtrieu!




Comment at: clang/lib/Analysis/CFG.cpp:74-75
 /// Returns true on constant values based around a single IntegerLiteral.
 /// Allow for use of parentheses, integer casts, and negative signs.
 static bool IsIntegerLiteralConstantExpr(const Expr *E) {
   // Allow parentheses





Comment at: clang/lib/Analysis/CFG.cpp:1015-1016
+  // which are an IntegerLiteral or a UnaryOperator and returns the value with
+  // all operations performed on it.
+  Optional getIntegerLiteralSubexpressionValue(const Expr *E) {
+




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

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

[PATCH] D132135: [clangd] Support hover on __func__ etc (PredefinedExpr)

[Christian Kandeler via Phabricator via cfe-commits]

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

Works fine for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132135

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

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

By the way: How do I change the commit message when using arc? It seems to 
ignore all subsequent changes other than in the code itself, and now the patch 
is in the repo with a misleading commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

[Vaibhav Yenamandra via Phabricator via cfe-commits]

vaibhav.y added a comment.

Thank you! I don't have merge access, will need your help for that :)

For the commit details, I'd like to use:

- Name: Vaibhav Yenamandra
- email address: vyenaman...@bloomberg.net


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131084

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

[PATCH] D131662: [clang] Try to improve diagnostics about uninitialized constexpr variables

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

tbaeder updated this revision to Diff 453664.

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

https://reviews.llvm.org/D131662

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp

Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -27,6 +27,10 @@
   constexpr int zero() const { return 0; }
 };
 
+constexpr int arr[]; // expected-error {{constexpr variable 'arr' must be initialized by a constant expression}}
+constexpr int arr2[2]; // expected-error {{constexpr variable 'arr2' must be initialized by a constant expression}}
+constexpr int arr3[2] = {};
+
 namespace DerivedToVBaseCast {
 
   struct U { int n; };
@@ -1298,7 +1302,7 @@
   void f() {
 extern constexpr int i; // expected-error {{constexpr variable declaration must be a definition}}
 constexpr int j = 0;
-constexpr int k; // expected-error {{default initialization of an object of const type}}
+constexpr int k; // expected-error {{constexpr variable 'k' must be initialized by a constant expression}}
   }
 
   extern const int q;
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
@@ -18,7 +18,7 @@
 
 // A variable declaration which uses the constexpr specifier shall have an
 // initializer and shall be initialized by a constant expression.
-constexpr int ni1; // expected-error {{default initialization of an object of const type 'const int'}}
+constexpr int ni1; // expected-error {{constexpr variable 'ni1' must be initialized by a constant expression}}
 constexpr struct C { C(); } ni2; // expected-error {{cannot have non-literal type 'const struct C'}} expected-note 3{{has no constexpr constructors}}
 constexpr double &ni3; // expected-error {{declaration of reference variable 'ni3' requires an initializer}}
 
Index: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
===
--- clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
+++ clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp
@@ -37,7 +37,7 @@
 #if __cplusplus <= 201402L && !defined(MS_ABI)
   // expected-error@-2 {{requires an initializer}}
 #else
-  // expected-error@-4 {{default initialization of an object of const}}
+  // expected-error@-4 {{constexpr variable 'mi2' must be initialized by a constant expression}}
 #endif
   mutable constexpr int mi3 = 3; // expected-error-re {{non-static data member cannot be constexpr{{$ expected-error {{'mutable' and 'const' cannot be mixed}}
 };
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -8053,19 +8053,29 @@
 return ExprError();
   }
   if (!ZeroInitializationFixit.empty()) {
-unsigned DiagID = diag::err_default_init_const;
-if (Decl *D = Entity.getDecl())
-  if (S.getLangOpts().MSVCCompat && D->hasAttr())
-DiagID = diag::ext_default_init_const;
+const Decl *D = Entity.getDecl();
+const VarDecl *VD = dyn_cast_or_null(D);
+QualType DestType = Entity.getType();
 
 // The initialization would have succeeded with this fixit. Since the fixit
 // is on the error, we need to build a valid AST in this case, so this isn't
 // handled in the Failed() branch above.
-QualType DestType = Entity.getType();
-S.Diag(Kind.getLocation(), DiagID)
-<< DestType << (bool)DestType->getAs()
-<< FixItHint::CreateInsertion(ZeroInitializationFixitLoc,
-  ZeroInitializationFixit);
+if (!DestType->getAs() && VD && VD->isConstexpr()) {
+  // Use a more useful diagnostic for constexpr variables.
+  S.Diag(Kind.getLocation(), diag::err_constexpr_var_requires_const_init)
+  << VD
+  << FixItHint::CreateInsertion(ZeroInitializationFixitLoc,
+ZeroInitializationFixit);
+} else {
+  unsigned DiagID = diag::err_default_init_const;
+  if (S.getLangOpts().MSVCCompat && D && D->hasAttr())
+DiagID = diag::ext_default_init_const;
+
+  S.Diag(Kind.getLocation(), DiagID)
+  << DestType << (bool)DestType->getAs()
+  << FixItHint::CreateInsertion(ZeroInitializationFixitLoc,
+ZeroInitializationFixit);
+}
   }
 
   if (getKind() == DependentSequence) {
@@ -9464,6 +9474,10 @@
 << Entity.getName();
   S.Diag(Entity.getDecl()->getLocation(), diag::note_previous_decl)
   

[PATCH] D131662: [clang] Try to improve diagnostics about uninitialized constexpr variables

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

tbaeder added a comment.

The new version also gets the new error message for `constexpr int foo[2];`.


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

https://reviews.llvm.org/D131662

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

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a subscriber: thieta.
tstellar added a comment.

@dblaikie Is there anything we need to do in the release branch for this still? 
cc @thieta


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117616

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

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D117616#3732173 , @tstellar wrote:

> @dblaikie Is there anything we need to do in the release branch for this 
> still? cc @thieta

Sorry, yeah, I'd tagged you over here: https://reviews.llvm.org/D118511#3717753 
for part of that discussion to see if we wanted to revert this from the release 
the same as we did the previous one... sorry this has dragged on so long, the 
second ABI fix (podness of structs with defaulted special members) and the 
warning (not wanting to warn/tell people to change things that were going to 
become a non-issue once the second ABI fix was in) aren't in yet, though the 
second ABI fix is getting some traction at least ( 
https://reviews.llvm.org/D119051 ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117616

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

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

@dblaikie Can you file a bug and add the 15.0.0 Release Milestone, so we don't 
forget to fix this in the release branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118511

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

[PATCH] D132140: [AMDGPU] Add builtin s_sendmsg_rtn_b{32|64}

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: b-sumner, arsenm, foad, kzhuravl, bcahoon.
Herald added subscribers: kosarev, kerbowa, t-tye, tpr, dstuttard, jvesely.
Herald added a project: All.
yaxunl requested review of this revision.
Herald added a subscriber: wdng.

https://reviews.llvm.org/D132140

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
  clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl


Index: clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl
===
--- /dev/null
+++ clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple amdgcn-- -target-cpu gfx1030 -verify=GFX10 -S -o - 
%s
+// RUN: %clang_cc1 -triple amdgcn-- -target-cpu gfx1100 -verify=GFX11 -S -o - 
%s
+
+typedef unsigned int uint;
+typedef unsigned long ulong;
+
+void test(global uint* out1, global ulong* out2, int x) {
+  *out1 = __builtin_amdgcn_s_sendmsg_rtn_b32(0); // GFX10-error 
{{'__builtin_amdgcn_s_sendmsg_rtn_b32' needs target feature gfx11-insts}}
+  *out2 = __builtin_amdgcn_s_sendmsg_rtn_b64(0); // GFX10-error 
{{'__builtin_amdgcn_s_sendmsg_rtn_b64' needs target feature gfx11-insts}}
+#if __has_builtin(__builtin_amdgcn_s_sendmsg_rtn_b32)
+  *out1 = __builtin_amdgcn_s_sendmsg_rtn_b32(x); // GFX11-error {{argument to 
'__builtin_amdgcn_s_sendmsg_rtn_b32' must be a constant integer}}
+  *out2 = __builtin_amdgcn_s_sendmsg_rtn_b64(x); // GFX11-error {{argument to 
'__builtin_amdgcn_s_sendmsg_rtn_b64' must be a constant integer}}
+#endif
+}
Index: clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
@@ -0,0 +1,30 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx1100 -S 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx1101 -S 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx1102 -S 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx1103 -S 
-emit-llvm -o - %s | FileCheck %s
+
+typedef unsigned int ushort;
+typedef unsigned int uint;
+typedef unsigned long ulong;
+
+// CHECK-LABEL: @test_s_sendmsg_rtn_b32(
+// CHECK: call i32 @llvm.amdgcn.s.sendmsg.rtn.i32(i32 0)
+void test_s_sendmsg_rtn_b32(global uint* out) {
+  *out = __builtin_amdgcn_s_sendmsg_rtn_b32(0);
+}
+
+// CHECK-LABEL: @test_s_sendmsg_rtn_b64(
+// CHECK: call i64 @llvm.amdgcn.s.sendmsg.rtn.i64(i32 0)
+void test_s_sendmsg_rtn_b64(global ulong* out) {
+  *out = __builtin_amdgcn_s_sendmsg_rtn_b64(0);
+}
+
+// Test mismatched argument and return types are handled.
+
+// CHECK-LABEL: @test_s_sendmsg_rtn_b64_mismatch(
+// CHECK: call i64 @llvm.amdgcn.s.sendmsg.rtn.i64(i32 0)
+// CHECK:  uitofp i64 %{{.*}} to double
+void test_s_sendmsg_rtn_b64_mismatch(global double* out) {
+  *out = __builtin_amdgcn_s_sendmsg_rtn_b64((ushort)0);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -17015,6 +17015,15 @@
 
 return Builder.CreateCall(F, {Ptr, Val, MemOrder, MemScope, IsVolatile});
   }
+  case AMDGPU::BI__builtin_amdgcn_s_sendmsg_rtn_b32:
+  case AMDGPU::BI__builtin_amdgcn_s_sendmsg_rtn_b64: {
+llvm::Value *Arg = EmitScalarExpr(E->getArg(0));
+llvm::Type *ResultType = ConvertType(E->getType());
+// s_sendmsg_rtn is mangled using return type only.
+Function *F =
+CGM.getIntrinsic(Intrinsic::amdgcn_s_sendmsg_rtn, {ResultType});
+return Builder.CreateCall(F, {Arg});
+  }
   default:
 return nullptr;
   }
Index: clang/include/clang/Basic/BuiltinsAMDGPU.def
===
--- clang/include/clang/Basic/BuiltinsAMDGPU.def
+++ clang/include/clang/Basic/BuiltinsAMDGPU.def
@@ -277,6 +277,9 @@
 TARGET_BUILTIN(__builtin_amdgcn_wmma_i32_16x16x16_iu8_w64, 
"V4iIbV4iIbV4iV4iIb", "nc", "gfx11-insts")
 TARGET_BUILTIN(__builtin_amdgcn_wmma_i32_16x16x16_iu4_w64, 
"V4iIbV2iIbV2iV4iIb", "nc", "gfx11-insts")
 
+TARGET_BUILTIN(__builtin_amdgcn_s_sendmsg_rtn_b32, "UiUIi", "n", "gfx11-insts")
+TARGET_BUILTIN(__builtin_amdgcn_s_sendmsg_rtn_b64, "UWiUIi", "n", 
"gfx11-insts")
+
 
//===--===//
 // Special builtins.
 
//===--===//


Index: clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl
===
--- /dev/null
+++ clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple amdgcn-- -target-cpu gfx1030 -verify=GFX10 -S -o - %s
+// RUN: %clang_

[PATCH] D132141: [X86] Emulate _rdrand64_step with two rdrand32 if it is 32bit

[Bing Yu via Phabricator via cfe-commits]

yubing created this revision.
Herald added a subscriber: pengfei.
Herald added a project: All.
yubing 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/D132141

Files:
  clang/lib/Headers/immintrin.h
  clang/test/CodeGen/X86/rdrand-builtins.c


Index: clang/test/CodeGen/X86/rdrand-builtins.c
===
--- clang/test/CodeGen/X86/rdrand-builtins.c
+++ clang/test/CodeGen/X86/rdrand-builtins.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -no-opaque-pointers -ffreestanding %s 
-triple=x86_64-unknown-unknown -target-feature +rdrnd -target-feature +rdseed 
-emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK,X64
-// RUN: %clang_cc1 -no-opaque-pointers -ffreestanding %s 
-triple=i386-unknown-unknown -target-feature +rdrnd -target-feature +rdseed 
-emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_cc1 -no-opaque-pointers -ffreestanding %s 
-triple=i386-unknown-unknown -target-feature +rdrnd -target-feature +rdseed 
-emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK,X32
 
 #include 
 
@@ -24,6 +24,29 @@
 // X64: call { i64, i32 } @llvm.x86.rdrand.64
 // X64: store i64
 }
+#else
+int rdrand64(unsigned long long *p) {
+// X32: @rdrand64
+// X32: [[RETVAL_I:%.*]] = alloca i32, align 4
+// X32: call { i32, i32 } @llvm.x86.rdrand.32
+// X32: store i32
+// X32: call { i32, i32 } @llvm.x86.rdrand.32
+// X32: store i32
+// X32: [[AND_I:%.*]] = and i32
+// X32: [[TOBOOL_I:%.*]] = icmp ne i32 [[AND_I]], 0
+// X32: br i1 [[TOBOOL_I]], label [[IF_THEN_I:%.*]], label [[IF_ELSE_I:%.*]]
+// X32: if.then.i:
+// X32: store i64
+// X32: store i32 1, i32* [[RETVAL_I]], align 4
+// X32: br label [[_RDRAND64_STEP_EXIT:%.*]]
+// X32: if.else.i:
+// X32: store i64 0
+// X32: store i32 0, i32* [[RETVAL_I]], align 4
+// X32: br label [[_RDRAND64_STEP_EXIT]]
+// X32: _rdrand64_step.exit:
+// X32: %{{.*}} = load i32, i32* [[RETVAL_I]], align 4
+  return _rdrand64_step(p);
+}
 #endif
 
 int rdseed16(unsigned short *p) {
Index: clang/lib/Headers/immintrin.h
===
--- clang/lib/Headers/immintrin.h
+++ clang/lib/Headers/immintrin.h
@@ -291,6 +291,22 @@
 {
   return (int)__builtin_ia32_rdrand64_step(__p);
 }
+#else
+// We need to emulate the functionality of 64-bit rdrand with 2 32-bit
+// rdrand instructions.
+static __inline__ int __attribute__((__always_inline__, __nodebug__, 
__target__("rdrnd")))
+_rdrand64_step(unsigned long long *__p)
+{
+  unsigned long long tmp;
+  if (__builtin_ia32_rdrand32_step((unsigned int *)&tmp) &
+  __builtin_ia32_rdrand32_step(((unsigned int *)&tmp) + 1)) {
+*__p = tmp;
+return 1;
+  } else {
+*__p = 0;
+return 0;
+  }
+}
 #endif
 #endif /* __RDRND__ */
 


Index: clang/test/CodeGen/X86/rdrand-builtins.c
===
--- clang/test/CodeGen/X86/rdrand-builtins.c
+++ clang/test/CodeGen/X86/rdrand-builtins.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -no-opaque-pointers -ffreestanding %s -triple=x86_64-unknown-unknown -target-feature +rdrnd -target-feature +rdseed -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK,X64
-// RUN: %clang_cc1 -no-opaque-pointers -ffreestanding %s -triple=i386-unknown-unknown -target-feature +rdrnd -target-feature +rdseed -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_cc1 -no-opaque-pointers -ffreestanding %s -triple=i386-unknown-unknown -target-feature +rdrnd -target-feature +rdseed -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK,X32
 
 #include 
 
@@ -24,6 +24,29 @@
 // X64: call { i64, i32 } @llvm.x86.rdrand.64
 // X64: store i64
 }
+#else
+int rdrand64(unsigned long long *p) {
+// X32: @rdrand64
+// X32: [[RETVAL_I:%.*]] = alloca i32, align 4
+// X32: call { i32, i32 } @llvm.x86.rdrand.32
+// X32: store i32
+// X32: call { i32, i32 } @llvm.x86.rdrand.32
+// X32: store i32
+// X32: [[AND_I:%.*]] = and i32
+// X32: [[TOBOOL_I:%.*]] = icmp ne i32 [[AND_I]], 0
+// X32: br i1 [[TOBOOL_I]], label [[IF_THEN_I:%.*]], label [[IF_ELSE_I:%.*]]
+// X32: if.then.i:
+// X32: store i64
+// X32: store i32 1, i32* [[RETVAL_I]], align 4
+// X32: br label [[_RDRAND64_STEP_EXIT:%.*]]
+// X32: if.else.i:
+// X32: store i64 0
+// X32: store i32 0, i32* [[RETVAL_I]], align 4
+// X32: br label [[_RDRAND64_STEP_EXIT]]
+// X32: _rdrand64_step.exit:
+// X32: %{{.*}} = load i32, i32* [[RETVAL_I]], align 4
+  return _rdrand64_step(p);
+}
 #endif
 
 int rdseed16(unsigned short *p) {
Index: clang/lib/Headers/immintrin.h
===
--- clang/lib/Headers/immintrin.h
+++ clang/lib/Headers/immintrin.h
@@ -291,6 +291,22 @@
 {
   return (int)__builtin_ia32_rdrand64_step(__p);
 }
+#else
+// We need to emulate the functionality of 64-bi

[PATCH] D131526: [OMPIRBuilder] Add support for safelen clause

[Michael Kruse via Phabricator via cfe-commits]

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

LGTM




Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3042
+
+  if (!(Simdlen == nullptr && Safelen == nullptr)) {
+// If both simdlen and safelen clauses are specified, the value of the

Consider simplifying this expression



Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3048
+// parameter. Therefore, use safelen only in the absence of simdlen.
+ConstantInt *VectorizeWidth = Simdlen == nullptr ? Safelen : Simdlen;
 addLoopMetadata(

psoni2628 wrote:
> Meinersbur wrote:
> > `safelen` should not mean the same as `llvm.loop.vectorize.width`. 
> > `safelen` could be unreasonably large to use as SIMD width or a 
> > non-power-of-2.
> > 
> > That being said, it's what `CGStmtOpenMP.cpp` does as well and I don't know 
> > any better way.
> IMO, this is more of a semantic analysis problem. For example, if it not 
> legal to have a `safelen` that is a non-power-of-2, then Clang should not let 
> this value proceed from semantic analysis. Maybe we could add a check in 
> `clang/lib/Sema/SemaOpenMP.cpp`, and fix the problem for both OMPIRBuilder 
> and the existing codegen support in `CGStmtOpenMP.cpp` in a different patch?
I think it's not a responsibility of Clang, but the optimizer. For instance, 
the information that "infinite" vector width is good (#pragma clang loop 
vectorize(assume_safety)` and `#pragma omp simd` without `safelen`) is passed 
as metadata (`addSimdMetadata`), but there is not metadata for conveying that 
only a specific vector length is safe.

There are other issues than just power-of-2, such as
 * too-large safelen (e.g. 128 when the target only has simd instructions of 
length 4). 
 * A vector width of 2 having better performance even though `safelen(2)` is 
used.
 * The LoopVectorize will not vectorize at all if it cannot conclude the 
legality of it when only `!{!"llvm.loop.vectorize.width", i32 4}` is specified, 
but not also the information that the user guarantees that it is safe to do so.



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

https://reviews.llvm.org/D131526

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

[PATCH] D132141: [X86] Emulate _rdrand64_step with two rdrand32 if it is 32bit

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/Headers/immintrin.h:301
+  unsigned long long tmp;
+  if (__builtin_ia32_rdrand32_step((unsigned int *)&tmp) &
+  __builtin_ia32_rdrand32_step(((unsigned int *)&tmp) + 1)) {

Should `&` be `&&`?



Comment at: clang/lib/Headers/immintrin.h:301
+  unsigned long long tmp;
+  if (__builtin_ia32_rdrand32_step((unsigned int *)&tmp) &
+  __builtin_ia32_rdrand32_step(((unsigned int *)&tmp) + 1)) {

craig.topper wrote:
> Should `&` be `&&`?
Can we avoid the pointer cast here? Use two unsigned ints and manually 
concatenate them to a 64-bit value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132141

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

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D117616#3731188 , @abrachet wrote:

> While digging around failures we were seeing related to this I found some 
> behavior that diverges from gcc. See https://godbolt.org/z/qe18Wh4jf

thanks for the bug report, yeah - looks like we need "fieldClass.isPod && 
!fieldClass.packed" . I'll send out another patch & update here with a 
reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117616

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

[PATCH] D132140: [AMDGPU] Add builtin s_sendmsg_rtn_b{32|64}

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

Following existing naming, it might make sense to rename "rtn_b32" --> "rtn" 
and "rtn_b64" --> "rtnl".


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

https://reviews.llvm.org/D132140

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

[PATCH] D132142: [analyzer] Prefer wrapping SymbolicRegions by ElementRegions

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

steakhal created this revision.
steakhal added reviewers: NoQ, martong, ASDenysPetrov, Szelethus, isuckatcs, 
vabridgers.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It turns out that in certain cases `SymbolRegions` are wrapped by
`ElementRegions`; in others, it's not. This discrepancy can cause the
analyzer not to recognize if the two regions are actually referring to
the same entity, which then can lead to unreachable paths discovered.

Consider this example:

  struct Node { int* ptr; };
  void with_structs(Node* n1) {
  Node c = *n1; // copy
  Node* n2 = &c;
  clang_analyzer_dump(*n1); // lazy...
  clang_analyzer_dump(*n2); // lazy...
  clang_analyzer_dump(n1->ptr); // rval(n1->ptr): reg_$2}.ptr>
  clang_analyzer_dump(n2->ptr); // rval(n2->ptr): reg_$1},0 S64b,struct Node}.ptr>
  clang_analyzer_eval(n1->ptr != n2->ptr); // UNKNOWN, bad!
  (void)(*n1);
  (void)(*n2);
  }

The copy of `n1` will insert a new binding to the store; but for doing
that it actually must create a `TypedValueRegion` which it could pass to
the `LazyCompoundVal`. Since the memregion in question is a
`SymbolicRegion` - which is untyped, it needs to first wrap it into an
`ElementRegion` basically implementing this untyped -> typed conversion
for the sake of passing it to the `LazyCompoundVal`.
So, this is why we have `Element{SymRegion{.}, 0,struct Node}` for `n1`.

The problem appears if the analyzer evaluates a read from the expression
`n1->ptr`. The same logic won't apply for `SymbolRegionValues`, since
they accept raw `SubRegions`, hence the `SymbolicRegion` won't be
wrapped into an `ElementRegion` in that case.

Later when we arrive at the equality comparison, we cannot prove that
they are equal.

For more details check the corresponding thread on discourse:
https://discourse.llvm.org/t/are-symbolicregions-really-untyped/64406

---

In this patch, I'm eagerly wrapping each `SymbolicRegion` by an
`ElementRegion`; basically canonicalizing to this form.
It seems reasonable to do so since any object can be thought of as a single
array of that object; so this should not make much of a difference.

The tests also underpin this assumption, as only a few were broken by
this change; and actually fixed a FIXME along the way.

About the second example, which does the same copy operation - but on
the heap - it will be fixed by the next patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132142

Files:
  clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/ctor.mm
  clang/test/Analysis/expr-inspection.c
  clang/test/Analysis/memory-model.cpp
  clang/test/Analysis/ptr-arith.c
  clang/test/Analysis/ptr-arith.cpp
  clang/test/Analysis/trivial-copy-struct.cpp

Index: clang/test/Analysis/trivial-copy-struct.cpp
===
--- /dev/null
+++ clang/test/Analysis/trivial-copy-struct.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+template  void clang_analyzer_dump(T);
+void clang_analyzer_warnIfReached();
+
+struct Node { int* ptr; };
+
+void copy_on_stack(Node* n1) {
+  Node tmp = *n1;
+  Node* n2 = &tmp;
+  clang_analyzer_dump(n1); // expected-warning {{&SymRegion{reg_$0}}}
+  clang_analyzer_dump(n2); // expected-warning {{&tmp}}
+
+  clang_analyzer_dump(n1->ptr); // expected-warning {{&SymRegion{reg_$1},0 S64b,struct Node}.ptr>}}}
+  clang_analyzer_dump(n2->ptr); // expected-warning {{&SymRegion{reg_$1},0 S64b,struct Node}.ptr>}}}
+
+  if (n1->ptr != n2->ptr)
+clang_analyzer_warnIfReached(); // unreachable
+  (void)(n1->ptr);
+  (void)(n2->ptr);
+}
+
+void copy_on_heap(Node* n1) {
+  Node* n2 = new Node(*n1);
+
+  clang_analyzer_dump(n1); // expected-warning {{&SymRegion{reg_$2}}}
+  clang_analyzer_dump(n2); // expected-warning {{&HeapSymRegion{conj_$1{Node *, LC1, S1855, #1
+
+  clang_analyzer_dump(n1->ptr); // expected-warning {{&SymRegion{reg_$3},0 S64b,struct Node}.ptr>}}}
+  clang_analyzer_dump(n2->ptr); // expected-warning {{Unknown}} FIXME: This should be the same as above.
+
+  if (n1->ptr != n2->ptr)
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} FIXME: This should not be reachable.
+  (void)(n1->ptr);
+  (void)(n2->ptr);
+}
Index: clang/test/Analysis/ptr-arith.cpp
===
---

[PATCH] D132143: [analyzer] LazyCompoundVals should be always bound as default bindings

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

steakhal created this revision.
steakhal added reviewers: NoQ, xazax.hun, ASDenysPetrov, martong, isuckatcs, 
vabridgers, Szelethus.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`LazyCompoundVals` should only appear as `default` bindings in the
store. This fixes the second case in this patch-stack.

Depends on: D132142 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132143

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/trivial-copy-struct.cpp


Index: clang/test/Analysis/trivial-copy-struct.cpp
===
--- clang/test/Analysis/trivial-copy-struct.cpp
+++ clang/test/Analysis/trivial-copy-struct.cpp
@@ -27,10 +27,10 @@
   clang_analyzer_dump(n2); // expected-warning {{&HeapSymRegion{conj_$1{Node 
*, LC1, S1855, #1
 
   clang_analyzer_dump(n1->ptr); // expected-warning {{&SymRegion{reg_$3},0 S64b,struct Node}.ptr>}}}
-  clang_analyzer_dump(n2->ptr); // expected-warning {{Unknown}} FIXME: This 
should be the same as above.
+  clang_analyzer_dump(n2->ptr); // expected-warning {{&SymRegion{reg_$3},0 S64b,struct Node}.ptr>}}}
 
   if (n1->ptr != n2->ptr)
-clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} FIXME: 
This should not be reachable.
+clang_analyzer_warnIfReached(); // unreachable
   (void)(n1->ptr);
   (void)(n2->ptr);
 }
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2400,7 +2400,11 @@
 
   // Clear out bindings that may overlap with this binding.
   RegionBindingsRef NewB = removeSubRegionBindings(B, cast(R));
-  return NewB.addBinding(BindingKey::Make(R, BindingKey::Direct), V);
+
+  // LazyCompoundVals should be always bound as 'default' bindings.
+  auto KeyKind = isa(V) ? BindingKey::Default
+ : BindingKey::Direct;
+  return NewB.addBinding(BindingKey::Make(R, KeyKind), V);
 }
 
 RegionBindingsRef


Index: clang/test/Analysis/trivial-copy-struct.cpp
===
--- clang/test/Analysis/trivial-copy-struct.cpp
+++ clang/test/Analysis/trivial-copy-struct.cpp
@@ -27,10 +27,10 @@
   clang_analyzer_dump(n2); // expected-warning {{&HeapSymRegion{conj_$1{Node *, LC1, S1855, #1
 
   clang_analyzer_dump(n1->ptr); // expected-warning {{&SymRegion{reg_$3},0 S64b,struct Node}.ptr>}}}
-  clang_analyzer_dump(n2->ptr); // expected-warning {{Unknown}} FIXME: This should be the same as above.
+  clang_analyzer_dump(n2->ptr); // expected-warning {{&SymRegion{reg_$3},0 S64b,struct Node}.ptr>}}}
 
   if (n1->ptr != n2->ptr)
-clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} FIXME: This should not be reachable.
+clang_analyzer_warnIfReached(); // unreachable
   (void)(n1->ptr);
   (void)(n2->ptr);
 }
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2400,7 +2400,11 @@
 
   // Clear out bindings that may overlap with this binding.
   RegionBindingsRef NewB = removeSubRegionBindings(B, cast(R));
-  return NewB.addBinding(BindingKey::Make(R, BindingKey::Direct), V);
+
+  // LazyCompoundVals should be always bound as 'default' bindings.
+  auto KeyKind = isa(V) ? BindingKey::Default
+ : BindingKey::Direct;
+  return NewB.addBinding(BindingKey::Make(R, KeyKind), V);
 }
 
 RegionBindingsRef
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132141: [X86] Emulate _rdrand64_step with two rdrand32 if it is 32bit

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: clang/lib/Headers/immintrin.h:301
+  unsigned long long tmp;
+  if (__builtin_ia32_rdrand32_step((unsigned int *)&tmp) &
+  __builtin_ia32_rdrand32_step(((unsigned int *)&tmp) + 1)) {

craig.topper wrote:
> craig.topper wrote:
> > Should `&` be `&&`?
> Can we avoid the pointer cast here? Use two unsigned ints and manually 
> concatenate them to a 64-bit value.
+1
```
unsigned int lo, hi;
if (__builtin_ia32_rdrand32_step(&lo) &&
__builtin_ia32_rdrand32_step(&hi)) {
  *p = ((unsigned long)hi << 32) | lo;
  return 1;
}
```



Comment at: clang/test/CodeGen/X86/rdrand-builtins.c:2
 // RUN: %clang_cc1 -no-opaque-pointers -ffreestanding %s 
-triple=x86_64-unknown-unknown -target-feature +rdrnd -target-feature +rdseed 
-emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK,X64
-// RUN: %clang_cc1 -no-opaque-pointers -ffreestanding %s 
-triple=i386-unknown-unknown -target-feature +rdrnd -target-feature +rdseed 
-emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK
+// RUN: %clang_cc1 -no-opaque-pointers -ffreestanding %s 
-triple=i386-unknown-unknown -target-feature +rdrnd -target-feature +rdseed 
-emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK,X32
 

X86 not X32 :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132141

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

[PATCH] D132140: [AMDGPU] Add builtin s_sendmsg_rtn_b{32|64}

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D132140#3732262 , @b-sumner wrote:

> Following existing naming, it might make sense to rename "rtn_b32" --> "rtn" 
> and "rtn_b64" --> "rtnl".

will modify. thanks.


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

https://reviews.llvm.org/D132140

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

[PATCH] D130788: [clang-repl] Disable building when LLVM_STATIC_LINK_CXX_STDLIB is ON.

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

In D130788#3731232 , @sunho wrote:

> In D130788#3730533 , @sbc100 wrote:
>
>> I'm not totally sure but I think the change is responsible for the 
>> emscripten integration bot failing `InterpreterTest.CatchException`: 
>> https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8807160007692150337/+/u/LLVM_regression/stdout
>>
>>   [==] Running 1 test from 1 test suite.
>>   [--] Global test environment set-up.
>>   [--] 1 test from InterpreterTest
>>   [ RUN  ] InterpreterTest.CatchException
>>   JIT session error: Symbols not found: [ __gxx_personality_v0, 
>> _ZSt9terminatev, _ZTVN10__cxxabiv117__class_type_infoE, 
>> __cxa_allocate_exception, __cxa_begin_catch, __cxa_end_catch, 
>> __cxa_free_exception, __cxa_throw ]
>>   Failure value returned from cantFail wrapped call
>>   Failed to materialize symbols: { (main, { _ZN16custom_exceptionC2EPKc, 
>> __clang_call_terminate, _ZTI16custom_exception, _ZTS16custom_exception, 
>> throw_exception }) }
>>   UNREACHABLE executed at 
>> /b/s/w/ir/cache/builder/emscripten-releases/llvm-project/llvm/include/llvm/Support/Error.h:786!
>>   Stack dump without symbol names (ensure you have llvm-symbolizer in your 
>> PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x2464413)[0x55cb14660413]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246236c)[0x55cb1465e36c]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x24648df)[0x55cb146608df]
>>   /lib/x86_64-linux-gnu/libpthread.so.0(+0x12980)[0x7fad2fab1980]
>>   /lib/x86_64-linux-gnu/libc.so.6(gsignal+0xc7)[0x7fad2eb0de87]
>>   /lib/x86_64-linux-gnu/libc.so.6(abort+0x141)[0x7fad2eb0f7f1]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x23f798f)[0x55cb145f398f]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9de35)[0x55cb14099e35]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9d597)[0x55cb14099597]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246d6be)[0x55cb146696be]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246e659)[0x55cb1466a659]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246ee40)[0x55cb1466ae40]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x247b2c3)[0x55cb146772c3]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x247ab42)[0x55cb14676b42]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x246559c)[0x55cb1466159c]
>>   /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xe7)[0x7fad2eaf0c87]
>>   
>> /b/s/w/ir/cache/builder/emscripten-releases/build/llvm-out/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests(+0x1e9ceda)[0x55cb14098eda]
>>
>> This started happening consistently after this change 
>> https://chromium.googlesource.com/emscripten-releases/+/584b2f531314d1e70cd5ebadcce7e015a6215c9a.
>>   The only CL in that list that looks related seems to be this one.
>
> Could you share the detailed build configuration of those bots?

All the step should be visible here: 
https://ci.chromium.org/ui/p/emscripten-releases/builders/ci/linux-test-suites/b8805531315340503553/steps

The most relevant one I guess would be:

https://logs.chromium.org/logs/emscripten-releases/buildbucket/cr-buildbucket/8805531315340503553/+/u/Build_LLVM/stdout

Here you can see LLVM being configured with:

  
subprocess.check_call(`/b/s/w/ir/cache/builder/emscripten-releases/cmake-3.21.3-linux-x86_64/bin/cmake
 -G Ninja -DPython3_EXECUTABLE=/b/s/w/ir/cache/vpython/68bda9/bin/python 
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_BUILD_TYPE=Release 
-DCMAKE_INSTALL_PREFIX=/b/s/w/ir/x/w/install 
-DCMA

[PATCH] D132140: [AMDGPU] Add builtin s_sendmsg_rtn_b{32|64}

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 453681.
yaxunl added a comment.

revised by Brian's comments


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

https://reviews.llvm.org/D132140

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
  clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl


Index: clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl
===
--- /dev/null
+++ clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple amdgcn-- -target-cpu gfx1030 -verify=GFX10 -S -o - 
%s
+// RUN: %clang_cc1 -triple amdgcn-- -target-cpu gfx1100 -verify=GFX11 -S -o - 
%s
+
+typedef unsigned int uint;
+typedef unsigned long ulong;
+
+void test(global uint* out1, global ulong* out2, int x) {
+  *out1 = __builtin_amdgcn_s_sendmsg_rtn(0); // GFX10-error 
{{'__builtin_amdgcn_s_sendmsg_rtn' needs target feature gfx11-insts}}
+  *out2 = __builtin_amdgcn_s_sendmsg_rtnl(0); // GFX10-error 
{{'__builtin_amdgcn_s_sendmsg_rtnl' needs target feature gfx11-insts}}
+#if __has_builtin(__builtin_amdgcn_s_sendmsg_rtn)
+  *out1 = __builtin_amdgcn_s_sendmsg_rtn(x); // GFX11-error {{argument to 
'__builtin_amdgcn_s_sendmsg_rtn' must be a constant integer}}
+#endif
+#if __has_builtin(__builtin_amdgcn_s_sendmsg_rtnl)
+  *out2 = __builtin_amdgcn_s_sendmsg_rtnl(x); // GFX11-error {{argument to 
'__builtin_amdgcn_s_sendmsg_rtnl' must be a constant integer}}
+#endif
+}
Index: clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/builtins-amdgcn-gfx11.cl
@@ -0,0 +1,30 @@
+// REQUIRES: amdgpu-registered-target
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx1100 -S 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx1101 -S 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx1102 -S 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-unknown-unknown -target-cpu gfx1103 -S 
-emit-llvm -o - %s | FileCheck %s
+
+typedef unsigned int ushort;
+typedef unsigned int uint;
+typedef unsigned long ulong;
+
+// CHECK-LABEL: @test_s_sendmsg_rtn(
+// CHECK: call i32 @llvm.amdgcn.s.sendmsg.rtn.i32(i32 0)
+void test_s_sendmsg_rtn(global uint* out) {
+  *out = __builtin_amdgcn_s_sendmsg_rtn(0);
+}
+
+// CHECK-LABEL: @test_s_sendmsg_rtnl(
+// CHECK: call i64 @llvm.amdgcn.s.sendmsg.rtn.i64(i32 0)
+void test_s_sendmsg_rtnl(global ulong* out) {
+  *out = __builtin_amdgcn_s_sendmsg_rtnl(0);
+}
+
+// Test mismatched argument and return types are handled.
+
+// CHECK-LABEL: @test_s_sendmsg_rtnl_mismatch(
+// CHECK: call i64 @llvm.amdgcn.s.sendmsg.rtn.i64(i32 0)
+// CHECK:  uitofp i64 %{{.*}} to double
+void test_s_sendmsg_rtnl_mismatch(global double* out) {
+  *out = __builtin_amdgcn_s_sendmsg_rtnl((ushort)0);
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -17015,6 +17015,15 @@
 
 return Builder.CreateCall(F, {Ptr, Val, MemOrder, MemScope, IsVolatile});
   }
+  case AMDGPU::BI__builtin_amdgcn_s_sendmsg_rtn:
+  case AMDGPU::BI__builtin_amdgcn_s_sendmsg_rtnl: {
+llvm::Value *Arg = EmitScalarExpr(E->getArg(0));
+llvm::Type *ResultType = ConvertType(E->getType());
+// s_sendmsg_rtn is mangled using return type only.
+Function *F =
+CGM.getIntrinsic(Intrinsic::amdgcn_s_sendmsg_rtn, {ResultType});
+return Builder.CreateCall(F, {Arg});
+  }
   default:
 return nullptr;
   }
Index: clang/include/clang/Basic/BuiltinsAMDGPU.def
===
--- clang/include/clang/Basic/BuiltinsAMDGPU.def
+++ clang/include/clang/Basic/BuiltinsAMDGPU.def
@@ -277,6 +277,9 @@
 TARGET_BUILTIN(__builtin_amdgcn_wmma_i32_16x16x16_iu8_w64, 
"V4iIbV4iIbV4iV4iIb", "nc", "gfx11-insts")
 TARGET_BUILTIN(__builtin_amdgcn_wmma_i32_16x16x16_iu4_w64, 
"V4iIbV2iIbV2iV4iIb", "nc", "gfx11-insts")
 
+TARGET_BUILTIN(__builtin_amdgcn_s_sendmsg_rtn, "UiUIi", "n", "gfx11-insts")
+TARGET_BUILTIN(__builtin_amdgcn_s_sendmsg_rtnl, "UWiUIi", "n", "gfx11-insts")
+
 
//===--===//
 // Special builtins.
 
//===--===//


Index: clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl
===
--- /dev/null
+++ clang/test/SemaOpenCL/builtins-amdgcn-gfx11.cl
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple amdgcn-- -target-cpu gfx1030 -verify=GFX10 -S -o - %s
+// RUN: %clang_cc1 -triple amdgcn-- -target-cpu gfx1100 -verify=GFX11 -S -o - %s
+
+typedef unsigned int uint;
+typedef unsigned long ulong;
+

[PATCH] D131175: [clangd] Use the "macro" semantic token for pre-defined identifiers

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D131175#3732163 , @ckandeler wrote:

> By the way: How do I change the commit message when using arc? It seems to 
> ignore all subsequent changes other than in the code itself, and now the 
> patch is in the repo with a misleading commit message.

Yeah that's unfortunate and I do it myself all the time :-(

When I remember, my workflow is to amend the commit message locally with git, 
and then `arc diff --verbatim` will push this into the phab description (in 
addition to the usual snapshotting behavior).
But often I forget and edit it in phab, and I don't know of a command to pull 
those edits down into my git repo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131175

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

[clang-tools-extra] 7f2a079 - [clangd] Fix a tsan failure in PreambleThrottle tests

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2022-08-18T18:36:01+02:00
New Revision: 7f2a079a122b5f3abd4afa6697e951616dca768b

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

LOG: [clangd] Fix a tsan failure in PreambleThrottle tests

Added: 


Modified: 
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp 
b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index 33c92b3425e8..fd56b0d75ae2 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1401,9 +1401,12 @@ TEST_F(TUSchedulerTests, PreambleThrottle) {
   }
   if (Invoke)
 Invoke();
-  if (Notify && ID == Notify->first) {
-Notify->second->notify();
-Notify.reset();
+  {
+std::lock_guard Lock(Mu);
+if (Notify && ID == Notify->first) {
+  Notify->second->notify();
+  Notify.reset();
+}
   }
   return ID;
 }



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

[PATCH] D129755: Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for your patience on the review! I've taken a cursory look through 
and I'm still thinking about the patch. I've not seen anything that's really 
worrying. But this is pretty dense stuff and @delesley has way more experience 
with TIL, so I'm hoping he might be able to take a peek as well.




Comment at: clang/docs/ThreadSafetyAnalysis.rst:935
+// Same as constructors, but without tag types. (Requires C++17 copy 
elision.)
+static MutexLocker Lock(Mutex *mu) ACQUIRE(mu) NO_THREAD_SAFETY_ANALYSIS {
+  return MutexLocker(mu);

aaronpuchert wrote:
> The `NO_THREAD_SAFETY_ANALYSIS` here is due to a false positive warning 
> because the scope is not destructed in the function body. Maybe we should 
> postpone documentation until this is resolved.
I don't know that there's a lot of value to documenting it if we have to 
disable thread safety analysis, so my instinct is to hold off on the 
documentation for the moment.



Comment at: clang/include/clang/Analysis/Analyses/ThreadSafetyTIL.h:435
 /// Used to implement lazy copy and rewriting strategies.
 class Future : public SExpr {
 public:

aaronpuchert wrote:
> By the way, I considered using this, but it didn't seem wholly appropriate. 
> Our placeholder can't be lazily evaluated, we simply don't know initially. 
> And later we do know and can just plug in the `VarDecl`.
Hmmm, naively, it seems like `FS_pending` is "we don't know initially" and 
`FS_done` is "now we do know and can plugin in the `VarDecl`". But I agree it 
seems a little different from lazy evaluation in that it's not a performance 
optimization.



Comment at: clang/include/clang/Analysis/Analyses/ThreadSafetyTraverse.h:629
+else
+  SS << "";
   }

aaronpuchert wrote:
> Perhaps no warning will ever print this, but I'm not entirely sure.
Should we assert here instead?



Comment at: clang/lib/Analysis/ThreadSafety.cpp:1538
   FactSet FSet;
+  SmallVector ConstructedObjects;
   LocalVariableMap::Context LVarCtx;

aaronpuchert wrote:
> This is essentially an `std::map`, but since 
> it only keeps objects between construction and assignment to a variable or 
> temporary destruction, I thought that a small vector would be more 
> appropriate. However, there is no guarantee that this doesn't grow in size 
> significantly: temporaries with trivial destructors (which don't show up in 
> the CFG) would never be removed from this list. That's unlikely for properly 
> annotated scopes, but not impossible.
Would it make more sense to use a `SmallDenseMap` here?



Comment at: clang/lib/Analysis/ThreadSafety.cpp:2422
+case CFGElement::TemporaryDtor: {
+  CFGTemporaryDtor TD = BI.castAs();
+





Comment at: clang/lib/Analysis/ThreadSafetyCommon.cpp:339-342
+if (const Expr *SelfArg = dyn_cast(Ctx->SelfArg))
+  return translate(SelfArg, Ctx->Prev);
+else
+  return cast(Ctx->SelfArg);




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129755

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

[PATCH] D132144: [Clang][BPF] Support record argument with direct values

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song created this revision.
yonghong-song added a reviewer: ast.
Herald added subscribers: pengfei, kristof.beyls.
Herald added a project: All.
yonghong-song requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently, record arguments are always passed by reference by allocating
space for record values in the caller. This is less efficient for 
small records which may take one or two registers. For example,
for x86_64 and aarch64, for a record size up to 16 bytes, the record
values can be passed by values directly on the registers.

This patch added BPF support of record argument with direct values
for up to 16 byte record size. If record size is 0, that record
will not take any register, which is the same behavior for x86_64
and aarch64. If the record size is greater than 16 bytes, the 
record argument will be passed by reference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132144

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/bpf-struct-argument.c
  clang/test/CodeGen/bpf-union-argument.c

Index: clang/test/CodeGen/bpf-union-argument.c
===
--- /dev/null
+++ clang/test/CodeGen/bpf-union-argument.c
@@ -0,0 +1,44 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang_cc1 -triple bpf -O2 -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+
+union t1 {};
+union t2 {
+  int a;
+  long b;
+};
+union t3 {
+  struct {
+int a;
+long b;
+  };
+  long c;
+};
+union t4 {
+  struct {
+long a;
+long b;
+long c;
+  };
+  long d;
+};
+
+int foo1(union t1 arg1, union t2 arg2) {
+// CHECK: define dso_local i32 @foo1(i64 %arg2.coerce)
+  return arg2.a;
+}
+
+int foo2(union t3 arg1, union t4 arg2) {
+// CHECK: define dso_local i32 @foo2([2 x i64] %arg1.coerce, ptr noundef byval(%union.t4) align 8 %arg2)
+  return arg1.a + arg2.a;
+
+}
+
+int foo3(void) {
+  union t1 tmp1 = {};
+  union t2 tmp2 = {};
+  union t3 tmp3 = {};
+  union t4 tmp4 = {};
+  return foo1(tmp1, tmp2) + foo2(tmp3, tmp4);
+// CHECK: call i32 @foo1(i64 %{{[a-zA-Z0-9]+}})
+// CHECK: call i32 @foo2([2 x i64] %{{[a-zA-Z0-9]+}}, ptr noundef byval(%union.t4) align 8 %tmp4)
+}
Index: clang/test/CodeGen/bpf-struct-argument.c
===
--- /dev/null
+++ clang/test/CodeGen/bpf-struct-argument.c
@@ -0,0 +1,36 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang_cc1 -triple bpf -O2 -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+
+struct t1 {};
+struct t2 {
+  int a;
+};
+struct t3 {
+  int a;
+  long b;
+};
+struct t4 {
+  long a;
+  long b;
+  long c;
+};
+
+int foo1(struct t1 arg1, struct t2 arg2) {
+// CHECK: define dso_local i32 @foo1(i32 %arg2.coerce)
+  return arg2.a;
+}
+
+int foo2(struct t3 arg1, struct t4 arg2) {
+// CHECK: define dso_local i32 @foo2([2 x i64] %arg1.coerce, ptr noundef byval(%struct.t4) align 8 %arg2)
+  return arg1.a + arg2.a;
+}
+
+int foo3(void) {
+  struct t1 tmp1 = {};
+  struct t2 tmp2 = {};
+  struct t3 tmp3 = {};
+  struct t4 tmp4 = {};
+  return foo1(tmp1, tmp2) + foo2(tmp3, tmp4);
+// CHECK: call i32 @foo1(i32 %{{[a-zA-Z0-9]+}})
+// CHECK: call i32 @foo2([2 x i64] %{{[a-zA-Z0-9]+}}, ptr noundef byval(%struct.t4) align 8 %tmp4)
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -11509,6 +11509,42 @@
 public:
   BPFABIInfo(CodeGenTypes &CGT) : DefaultABIInfo(CGT) {}
 
+  ABIArgInfo classifyArgumentType(QualType Ty) const {
+Ty = useFirstFieldIfTransparentUnion(Ty);
+
+if (isAggregateTypeForABI(Ty)) {
+  uint64_t Bits = getContext().getTypeSize(Ty);
+  if (Bits == 0)
+return ABIArgInfo::getIgnore();
+
+  // If the aggregate needs 1 or 2 registers, do not use reference.
+  if (Bits <= 128) {
+llvm::Type *CoerceTy;
+if (Bits <= 64) {
+  CoerceTy =
+  llvm::IntegerType::get(getVMContext(), llvm::alignTo(Bits, 8));
+} else {
+  llvm::Type *RegTy = llvm::IntegerType::get(getVMContext(), 64);
+  CoerceTy = llvm::ArrayType::get(RegTy, 2);
+}
+return ABIArgInfo::getDirect(CoerceTy);
+  } else {
+return getNaturalAlignIndirect(Ty);
+  }
+}
+
+if (const EnumType *EnumTy = Ty->getAs())
+  Ty = EnumTy->getDecl()->getIntegerType();
+
+ASTContext &Context = getContext();
+if (const auto *EIT = Ty->getAs())
+  if (EIT->getNumBits() > Context.getTypeSize(Context.Int128Ty))
+return getNaturalAlignIndirect(Ty);
+
+return (isPromotableIntegerTypeForABI(Ty) ? ABIArgInfo::getExtend(Ty)
+  : ABIArgInfo::getDirect());
+  }
+
   ABIArgInfo classifyReturnType(QualType RetTy) const {
 if (RetTy->isVoidType())
   return ABIArgInfo::getIgnore();

[PATCH] D131662: [clang] Try to improve diagnostics about uninitialized constexpr variables

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, though you should add a release note.




Comment at: clang/lib/Sema/SemaInit.cpp:8057
+const Decl *D = Entity.getDecl();
+const VarDecl *VD = dyn_cast_or_null(D);
+QualType DestType = Entity.getType();





Comment at: clang/lib/Sema/SemaInit.cpp:8063
 // handled in the Failed() branch above.
-QualType DestType = Entity.getType();
-S.Diag(Kind.getLocation(), DiagID)
-<< DestType << (bool)DestType->getAs()
-<< FixItHint::CreateInsertion(ZeroInitializationFixitLoc,
-  ZeroInitializationFixit);
+if (!DestType->getAs() && VD && VD->isConstexpr()) {
+  // Use a more useful diagnostic for constexpr variables.





Comment at: clang/lib/Sema/SemaInit.cpp:9477
 << Entity.getName();
+} else if (const auto *VD = dyn_cast_if_present(Entity.getDecl());
+   VD && VD->isConstexpr()) {

Oh, neato, I see `dyn_cast_or_null` was deprecated...


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

https://reviews.llvm.org/D131662

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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:94
 llvm::Function *F, const FunctionDecl *FD) {
-  if (HLSLShaderAttr *ShaderAttr = FD->getAttr()) {
-const StringRef ShaderAttrKindStr = "dx.shader";
-F->addFnAttr(ShaderAttrKindStr,
- ShaderAttr->ConvertShaderTypeToStr(ShaderAttr->getType()));
+  HLSLShaderAttr *ShaderAttr = FD->getAttr();
+  assert(ShaderAttr && "All entry functions must have a HLSLShaderAttr");





Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:107-108
+CGM.getIntrinsic(Intrinsic::dx_flattened_thread_id_in_group);
+CallInst *CI = B.CreateCall(FunctionCallee(DxGroupIndex));
+return CI;
+  }





Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:122-141
+  // 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(EntryFn, FD);
+





Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:135
+  llvm::SmallVector Args;
+  for (const auto Param : FD->parameters()) {
+Args.push_back(emitInputSemantic(B, *Param));





Comment at: clang/lib/CodeGen/CGHLSLRuntime.h:55
 
-  void setHLSLFunctionAttributes(llvm::Function *, const FunctionDecl *);
+  void setHLSLEntryAttributes(llvm::Function *, const FunctionDecl *);
+

These parameters should have names so people don't think the parameters aren't 
used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131203

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

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 453702.
wyt marked 2 inline comments as done.
wyt added a comment.

Address comments. Unable to rename `check/runDataflowOnCFG` to 
`check/runDataflow` currently due to ambiguous use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> &BlockStates;
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [&PostVisitStmt, &Context](
-   const CFGStmt &Stmt,
-   const TypeErasedDataflowAnalysisState &State) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [&PostVisitCFG, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto &BlockStates = *MaybeBlockStates;
@@ -141,6 +153,33 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[&PostVisitStmt](ASTContext &Context, const CFGElement &Element,
+ const TypeErasedDataflowAnalysisState &State) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +196,9 @@
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [&VerifyResults](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +219,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [&Results,
-   &Annotations](const clang::CFGStmt &Stmt,
+   &Annotations](const clang::CFGElement &Element,
  const TypeErasedDataflowAnalysisState &State) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to n

[PATCH] D131616: [clang][dataflow] Generalise match switch utility to other AST types and add a `CFGMatchSwitch` which currently handles `CFGStmt` and `CFGInitializer`.

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 453704.
wyt marked 5 inline comments as done.
wyt added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131616

Files:
  clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
@@ -5,12 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
-//
-//  This file defines a simplistic version of Constant Propagation as an example
-//  of a forward, monotonic dataflow analysis. The analysis tracks all
-//  variables in the scope, but lacks escape analysis.
-//
-//===--===//
 
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "TestingSupport.h"
Index: clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
===
--- clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_unittest(ClangAnalysisFlowSensitiveTests
+  CFGMatchSwitchTest.cpp
   ChromiumCheckModelTest.cpp
   DataflowAnalysisContextTest.cpp
   DataflowEnvironmentTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
@@ -0,0 +1,132 @@
+//===- unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace dataflow;
+using namespace ast_matchers;
+
+namespace {
+// State for tracking the number of matches on each kind of CFGElement by the
+// CFGMatchSwitch. Currently only tracks CFGStmt and CFGInitializer.
+struct CFGElementMatches {
+  unsigned StmtMatches = 0;
+  unsigned InitializerMatches = 0;
+};
+
+// Returns a match switch that counts the number of local variables
+// (singly-declared) and fields initialized to the integer literal 42.
+auto buildCFGMatchSwitch() {
+  return CFGMatchSwitchBuilder()
+  .CaseOfCFGStmt(
+  declStmt(hasSingleDecl(
+  varDecl(hasInitializer(integerLiteral(equals(42)),
+  [](const DeclStmt *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.StmtMatches++; })
+  .CaseOfCFGInit(
+  cxxCtorInitializer(withInitializer(integerLiteral(equals(42,
+  [](const CXXCtorInitializer *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.InitializerMatches++; })
+  .Build();
+}
+
+// Runs the match switch `MS` on the control flow graph generated from `Code`,
+// tracking information in state `S`. For simplicity, this test utility is
+// restricted to CFGs with a single control flow block (excluding entry and
+// exit blocks) - generated by `Code` with sequential flow (i.e. no branching).
+//
+// Requirements:
+//
+//  `Code` must contain a function named `f`, the body of this function will be
+//  used to generate the CFG.
+template 
+void applySwitchToCode(CFGMatchSwitch &MS, State &S,
+   llvm::StringRef Code) {
+  auto Unit = tooling::buildASTFromCodeWithArgs(Code, {"-Wno-unused-value"});
+  auto &Ctx = Unit->getASTContext();
+  const auto *F = selectFirst(
+  "f", match(functionDecl(isDefinition(), hasName("f")).bind("f"), Ctx));
+
+  CFG::BuildOptions BO;
+  BO.AddInitializers = true;
+
+  auto CFG = CFG::buildCFG(F, F->getBody(), &Ctx, BO);
+  auto CFGBlock = *CFG->getEntry().succ_begin();
+  for (auto &Elt : CFGBlock->Elements) {
+MS(Elt, Ctx, S

[PATCH] D132147: [clang][dataflow] Refactor `TestingSupport.h`

[weiyi via Phabricator via cfe-commits]

wyt created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a project: All.
wyt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Add `AnalysisArguments` struct for `checkDataflow`.

- Remove compulsory binding from statement to annotations. Instead, 
`checkDataflow` in the most general form takes a `VerifyResults` callback which 
takes as input an `AnalysisData` struct. This struct contains the data 
structures generated by the analysis that can then be tested. We then introduce 
two overloads/wrappers of `checkDataflow` for different mechanisms of testing - 
one which exposes annotation line numbers and is not restricted to statements, 
and the other which exposes states computed after annotated statements. In the 
future, we should look at retrieving the analysis states for constructs other 
than statements.

Depends On D131616 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132147

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1240,43 +1240,45 @@
 /*IgnoreSmartPointerDereference=*/true};
 std::vector Diagnostics;
 llvm::Error Error = checkDataflow(
-SourceCode, FuncMatcher,
-[Options](ASTContext &Ctx, Environment &) {
-  return UncheckedOptionalAccessModel(Ctx, Options);
+/*AA:AnalysisArguments=*/{
+.Code = SourceCode,
+.TargetFuncMatcher = FuncMatcher,
+.MakeAnalysis =
+[Options](ASTContext &Ctx, Environment &) {
+  return UncheckedOptionalAccessModel(Ctx, Options);
+},
+.PostVisitCFG =
+[&Diagnostics,
+ Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+ASTContext &Ctx, const CFGElement &Elt,
+const TypeErasedDataflowAnalysisState &State) mutable {
+  auto Stmt = Elt.getAs();
+  if (!Stmt) {
+return;
+  }
+  auto StmtDiagnostics =
+  Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env);
+  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
+},
+.ASTBuildArgs = {"-fsyntax-only", "-std=c++17",
+ "-Wno-undefined-inline"},
+.ASTBuildVirtualMappedFiles = FileContents,
 },
-[&Diagnostics, Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
-ASTContext &Ctx, const CFGStmt &Stmt,
-const TypeErasedDataflowAnalysisState &State) mutable {
-  auto StmtDiagnostics =
-  Diagnoser.diagnose(Ctx, Stmt.getStmt(), State.Env);
-  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
-},
-[&Diagnostics](AnalysisData AnalysisData) {
-  auto &SrcMgr = AnalysisData.ASTCtx.getSourceManager();
-
+/*VerifyResults=*/[&Diagnostics](llvm::DenseMap
+ &Annotations,
+ AnalysisData &AD) {
   llvm::DenseSet AnnotationLines;
-  for (const auto &Pair : AnalysisData.Annotations) {
-auto *Stmt = Pair.getFirst();
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getBeginLoc()));
-// We add both the begin and end locations, so that if the
-// statement spans multiple lines then the test will fail.
-//
-// FIXME: Going forward, we should change this to instead just
-// get the single line number from the annotation itself, rather
-// than looking at the statement it's attached to.
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getEndLoc()));
+  for (const auto &[Line, _] : Annotations) {
+AnnotationLines.insert(Line);
   }
-
+  auto &SrcMgr = AD.ASTCtx.getSourceManager();
   llvm::DenseSet DiagnosticLines;
   for (SourceLocation &Loc : Diagnostics) {
 DiagnosticLines.insert(SrcMgr.getPresumedLineNumber(Loc));
   }
 
   EXPECT_THAT(DiagnosticLines, ContainerEq(AnnotationLines));
-},
-{"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}, FileContents);
+});
 if (Error)
   FAIL() << llvm::toString(std::move(Error));
   }
Index: clang/unittests/Analy

[PATCH] D130207: [HLSL] Move DXIL validation version out of ModuleFlags

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

A few nitpicks, otherwise this looks good.




Comment at: llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp:61
 
-static void cleanModuleFlags(Module &M) {
-  constexpr StringLiteral DeadKeys[] = {ValVerKey};
-  // Collect DeadKeys in ModuleFlags.
-  StringSet<> DeadKeySet;
-  for (auto &Key : DeadKeys) {
-if (M.getModuleFlag(Key))
-  DeadKeySet.insert(Key);
-  }
-  if (DeadKeySet.empty())
-return;
-
-  SmallVector ModuleFlags;
-  M.getModuleFlagsMetadata(ModuleFlags);
-  NamedMDNode *MDFlags = M.getModuleFlagsMetadata();
-  MDFlags->eraseFromParent();
-  // Add ModuleFlag which not dead.
-  for (auto &Flag : ModuleFlags) {
-StringRef Key = Flag.Key->getString();
-if (DeadKeySet.contains(Key))
-  continue;
-M.addModuleFlag(Flag.Behavior, Key, Flag.Val);
-  }
-}
-
-static void cleanModule(Module &M) { cleanModuleFlags(M); }
+static void cleanModule(Module &M) {}
 

Better to remove this function if it is empty. If we need it in the future we 
can re-introduce it.



Comment at: llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp:80
 bool DXILTranslateMetadata::runOnModule(Module &M) {
-  if (MDNode *ValVerMD = cast_or_null(M.getModuleFlag(ValVerKey))) {
-auto ValVer = loadDXILValidatorVersion(ValVerMD);
+  if (auto *ValVerMD = M.getNamedMetadata(ValVerKey)) {
+auto ValVer = loadDXILValidatorVersion(ValVerMD->getOperand(0));

nit: LLVM's coding conventions discourage blanket use of `auto`. This one is a 
bit on the edge of the convention. My preference would be for this to have the 
type because I think that is more clear.

see: 
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable



Comment at: llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp:81
+  if (auto *ValVerMD = M.getNamedMetadata(ValVerKey)) {
+auto ValVer = loadDXILValidatorVersion(ValVerMD->getOperand(0));
 if (!ValVer.empty())

This should definitely have the type, not auto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130207

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

[PATCH] D132147: [clang][dataflow] Refactor `TestingSupport.h`

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 453708.
wyt added a comment.

Small fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132147

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1240,43 +1240,45 @@
 /*IgnoreSmartPointerDereference=*/true};
 std::vector Diagnostics;
 llvm::Error Error = checkDataflow(
-SourceCode, FuncMatcher,
-[Options](ASTContext &Ctx, Environment &) {
-  return UncheckedOptionalAccessModel(Ctx, Options);
+/*AA:AnalysisArguments=*/{
+.Code = SourceCode,
+.TargetFuncMatcher = FuncMatcher,
+.MakeAnalysis =
+[Options](ASTContext &Ctx, Environment &) {
+  return UncheckedOptionalAccessModel(Ctx, Options);
+},
+.PostVisitCFG =
+[&Diagnostics,
+ Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+ASTContext &Ctx, const CFGElement &Elt,
+const TypeErasedDataflowAnalysisState &State) mutable {
+  auto Stmt = Elt.getAs();
+  if (!Stmt) {
+return;
+  }
+  auto StmtDiagnostics =
+  Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env);
+  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
+},
+.ASTBuildArgs = {"-fsyntax-only", "-std=c++17",
+ "-Wno-undefined-inline"},
+.ASTBuildVirtualMappedFiles = FileContents,
 },
-[&Diagnostics, Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
-ASTContext &Ctx, const CFGStmt &Stmt,
-const TypeErasedDataflowAnalysisState &State) mutable {
-  auto StmtDiagnostics =
-  Diagnoser.diagnose(Ctx, Stmt.getStmt(), State.Env);
-  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
-},
-[&Diagnostics](AnalysisData AnalysisData) {
-  auto &SrcMgr = AnalysisData.ASTCtx.getSourceManager();
-
+/*VerifyResults=*/[&Diagnostics](llvm::DenseMap
+ &Annotations,
+ AnalysisData &AD) {
   llvm::DenseSet AnnotationLines;
-  for (const auto &Pair : AnalysisData.Annotations) {
-auto *Stmt = Pair.getFirst();
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getBeginLoc()));
-// We add both the begin and end locations, so that if the
-// statement spans multiple lines then the test will fail.
-//
-// FIXME: Going forward, we should change this to instead just
-// get the single line number from the annotation itself, rather
-// than looking at the statement it's attached to.
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getEndLoc()));
+  for (const auto &[Line, _] : Annotations) {
+AnnotationLines.insert(Line);
   }
-
+  auto &SrcMgr = AD.ASTCtx.getSourceManager();
   llvm::DenseSet DiagnosticLines;
   for (SourceLocation &Loc : Diagnostics) {
 DiagnosticLines.insert(SrcMgr.getPresumedLineNumber(Loc));
   }
 
   EXPECT_THAT(DiagnosticLines, ContainerEq(AnnotationLines));
-},
-{"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}, FileContents);
+});
 if (Error)
   FAIL() << llvm::toString(std::move(Error));
   }
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -56,47 +56,130 @@
 
 namespace test {
 
-// Returns assertions based on annotations that are present after statements in
-// `AnnotatedCode`.
-llvm::Expected>
-buildStatementToAnnotationMapping(const FunctionDecl *Func,
-  llvm::Annotations AnnotatedCode);
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisArguments {
+  /// Input code that is analyzed.
+  llvm::StringRef Code;
+  /// The body of the function which matches this matcher is analyzed.
+  ast_matchers::internal::Matcher TargetFuncMatcher;
+  /// The analysis to be ru

[PATCH] D127641: [clang-cl][MSVC] Enable /Zc:alignedNew for C++17 and /Zc:sizedDealloc by default

[Stephen Long via Phabricator via cfe-commits]

steplong added a comment.

@thakis @hans What do you both think of the latest version? I would like to get 
this merged


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

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