[PATCH] D150997: [llvm] Split out DenseMapInfo specialization

2023-06-21 Thread Elliot Goodrich via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcea0eea28e71: [llvm] Split out DenseMapInfovariant 
specialization (authored by IncludeGuardian).
Herald added a project: Flang.

Changed prior to commit:
  https://reviews.llvm.org/D150997?vs=532038=533470#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150997

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
  flang/include/flang/Optimizer/HLFIR/HLFIROps.h
  llvm/include/llvm/ADT/DenseMapInfo.h
  llvm/include/llvm/ADT/DenseMapInfoVariant.h
  llvm/include/llvm/CodeGen/CallingConvLower.h
  llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
  llvm/include/llvm/Object/DXContainer.h
  llvm/include/llvm/Transforms/Scalar/SROA.h
  llvm/unittests/ADT/DenseMapTest.cpp
  mlir/include/mlir/IR/AsmState.h
  mlir/include/mlir/Transforms/SROA.h

Index: mlir/include/mlir/Transforms/SROA.h
===
--- mlir/include/mlir/Transforms/SROA.h
+++ mlir/include/mlir/Transforms/SROA.h
@@ -13,6 +13,7 @@
 #include "mlir/Interfaces/MemorySlotInterfaces.h"
 #include "mlir/Support/LogicalResult.h"
 #include "llvm/ADT/Statistic.h"
+#include 
 
 namespace mlir {
 
Index: mlir/include/mlir/IR/AsmState.h
===
--- mlir/include/mlir/IR/AsmState.h
+++ mlir/include/mlir/IR/AsmState.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/StringMap.h"
 
 #include 
+#include 
 
 namespace mlir {
 class AsmResourcePrinter;
Index: llvm/unittests/ADT/DenseMapTest.cpp
===
--- llvm/unittests/ADT/DenseMapTest.cpp
+++ llvm/unittests/ADT/DenseMapTest.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseMapInfo.h"
+#include "llvm/ADT/DenseMapInfoVariant.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
Index: llvm/include/llvm/Transforms/Scalar/SROA.h
===
--- llvm/include/llvm/Transforms/Scalar/SROA.h
+++ llvm/include/llvm/Transforms/Scalar/SROA.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/IR/ValueHandle.h"
+#include 
 #include 
 
 namespace llvm {
Index: llvm/include/llvm/Object/DXContainer.h
===
--- llvm/include/llvm/Object/DXContainer.h
+++ llvm/include/llvm/Object/DXContainer.h
@@ -21,6 +21,7 @@
 #include "llvm/Support/Error.h"
 #include "llvm/Support/MemoryBufferRef.h"
 #include "llvm/TargetParser/Triple.h"
+#include 
 
 namespace llvm {
 namespace object {
Index: llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
===
--- llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
+++ llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
@@ -22,6 +22,7 @@
 #include "llvm/ExecutionEngine/Orc/ThreadSafeModule.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ThreadPool.h"
+#include 
 
 namespace llvm {
 namespace orc {
Index: llvm/include/llvm/CodeGen/CallingConvLower.h
===
--- llvm/include/llvm/CodeGen/CallingConvLower.h
+++ llvm/include/llvm/CodeGen/CallingConvLower.h
@@ -19,6 +19,8 @@
 #include "llvm/CodeGen/TargetCallingConv.h"
 #include "llvm/IR/CallingConv.h"
 #include "llvm/Support/Alignment.h"
+#include 
+#include 
 
 namespace llvm {
 
Index: llvm/include/llvm/ADT/DenseMapInfoVariant.h
===
--- /dev/null
+++ llvm/include/llvm/ADT/DenseMapInfoVariant.h
@@ -0,0 +1,71 @@
+//===- DenseMapInfoVariant.h - Type traits for DenseMap *- C++ -*-===//
+//
+// 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
+//
+//===--===//
+///
+/// \file
+/// This file defines DenseMapInfo traits for DenseMap>.
+///
+//===--===//
+
+#ifndef LLVM_ADT_DENSEMAPINFOVARIANT_H
+#define LLVM_ADT_DENSEMAPINFOVARIANT_H
+
+#include "llvm/ADT/DenseMapInfo.h"
+#include 
+#include 
+
+namespace llvm {
+
+// Provide DenseMapInfo for variants whose all alternatives have DenseMapInfo.
+template  struct DenseMapInfo> {
+  using Variant = std::variant;
+  using FirstT = std::variant_alternative_t<0, Variant>;
+
+  static inline Variant getEmptyKey() {
+return Variant(std::in_place_index<0>, DenseMapInfo::getEmptyKey());
+  }
+
+  static inline Variant getTombstoneKey() {
+return Variant(std::in_place_index<0>,
+   DenseMapInfo::getTombstoneKey());
+  }
+
+  static unsigned getHashValue(const Variant ) {
+

[clang-tools-extra] cea0eea - [llvm] Split out DenseMapInfo specialization

2023-06-21 Thread Elliot Goodrich via cfe-commits

Author: Elliot Goodrich
Date: 2023-06-22T06:50:54+01:00
New Revision: cea0eea28e71204bf8543ca94dbf185cbf597ca5

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

LOG: [llvm] Split out DenseMapInfo specialization

Remove the `DenseMapInfo>` variant out from
`llvm/ADT/DenseMapInfo.h` into a separate header
`llvm/ADT/DenseMapInfoVariant.h`

This allows us to remove the `` include, which is being
transitively and unncessary included in all translation units that
include `llvm/ADT/DenseMap.h`.

There have been similar changes to move out specializations for

* `APInt.h` fd7e309e02fd226b0390888388ed732608e52c73 and
* `StringRef.h`/`ArrayRef.h`
  983565a6fe4a9f40c7caf82b65c650c20dbcc104

to reduce the compilation time. As we are unable to move the
specialization into ``, we create a separate
`DenseMapInfoVariant.h` header that can be used by anyone who needs this
specialization.

This reduces the total number of preprocessing tokens across the LLVM
source files in lib from (roughly) 1,964,876,961 to 1,936,551,496 - a
reduction of ~1.44%. This should result in a small improvement in
compilation time.

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

Added: 
llvm/include/llvm/ADT/DenseMapInfoVariant.h

Modified: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
flang/include/flang/Optimizer/HLFIR/HLFIROps.h
llvm/include/llvm/ADT/DenseMapInfo.h
llvm/include/llvm/CodeGen/CallingConvLower.h
llvm/include/llvm/ExecutionEngine/Orc/LLJIT.h
llvm/include/llvm/Object/DXContainer.h
llvm/include/llvm/Transforms/Scalar/SROA.h
llvm/unittests/ADT/DenseMapTest.cpp
mlir/include/mlir/IR/AsmState.h
mlir/include/mlir/Transforms/SROA.h

Removed: 




diff  --git 
a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h 
b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
index 0f7fd1cfafa6b..48b018b382ba5 100644
--- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
+++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
@@ -26,10 +26,12 @@
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/DenseMapInfoVariant.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringMap.h"
 #include 
 #include 
+#include 
 #include 
 
 namespace llvm {

diff  --git a/flang/include/flang/Optimizer/HLFIR/HLFIROps.h 
b/flang/include/flang/Optimizer/HLFIR/HLFIROps.h
index 26bfc9a805dcc..ab426256c5aa5 100644
--- a/flang/include/flang/Optimizer/HLFIR/HLFIROps.h
+++ b/flang/include/flang/Optimizer/HLFIR/HLFIROps.h
@@ -19,6 +19,7 @@
 #include "mlir/IR/PatternMatch.h"
 #include "mlir/Interfaces/InferTypeOpInterface.h"
 #include "mlir/Interfaces/SideEffectInterfaces.h"
+#include 
 
 #include "flang/Optimizer/HLFIR/HLFIROpInterfaces.h.inc"
 #define GET_OP_CLASSES

diff  --git a/llvm/include/llvm/ADT/DenseMapInfo.h 
b/llvm/include/llvm/ADT/DenseMapInfo.h
index 2c227be0082ab..5b7dce7b53c62 100644
--- a/llvm/include/llvm/ADT/DenseMapInfo.h
+++ b/llvm/include/llvm/ADT/DenseMapInfo.h
@@ -20,7 +20,6 @@
 #include 
 #include 
 #include 
-#include 
 
 namespace llvm {
 
@@ -234,6 +233,14 @@ struct DenseMapInfo> {
 SecondInfo::getHashValue(PairVal.second));
   }
 
+  // Expose an additional function intended to be used by other
+  // specializations of DenseMapInfo without needing to know how
+  // to combine hash values manually
+  static unsigned getHashValuePiecewise(const T , const U ) {
+return detail::combineHashValue(FirstInfo::getHashValue(First),
+SecondInfo::getHashValue(Second));
+  }
+
   static bool isEqual(const Pair , const Pair ) {
 return FirstInfo::isEqual(LHS.first, RHS.first) &&
SecondInfo::isEqual(LHS.second, RHS.second);
@@ -290,52 +297,6 @@ template  struct 
DenseMapInfo> {
   }
 };
 
-// Provide DenseMapInfo for variants whose all alternatives have DenseMapInfo.
-template  struct DenseMapInfo> {
-  using Variant = std::variant;
-  using FirstT = std::variant_alternative_t<0, Variant>;
-
-  static inline Variant getEmptyKey() {
-return Variant(std::in_place_index<0>, 
DenseMapInfo::getEmptyKey());
-  }
-
-  static inline Variant getTombstoneKey() {
-return Variant(std::in_place_index<0>,
-   DenseMapInfo::getTombstoneKey());
-  }
-
-  static unsigned getHashValue(const Variant ) {
-return std::visit(
-[](auto &) {
-  using T = std::decay_t;
-  // Include index in hash to make sure same value as 
diff erent
-  // alternatives don't collide.
-  return detail::combineHashValue(
-  

[PATCH] D143241: [Clang] Reset FP options before function instantiations

2023-06-21 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added inline comments.



Comment at: clang/lib/Parse/ParseTemplate.cpp:1736
+  // point of the template definition.
+  Actions.resetFPOptions(LPT.FPO);
+

rjmccall wrote:
> Ah, is this bug specific to the MSVC-compatible template logic?
Exactly. This is MSVC bug-or-feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143241

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


[PATCH] D153228: [clang-format] Fixed bad performance with enabled qualifier fixer.

2023-06-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.

Thank you for the detailed analysis


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153228

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


[PATCH] D152671: [doc][LoongArch] Add missed release note about `ual` feature addition

2023-06-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/docs/ReleaseNotes.rst:164
+* An target feature ``ual`` is introduced to allow unaligned memory accesses 
and
+  this feature is enabled by default on generic 64bit processors.
 

xen0n wrote:
> nit: "for generic 64-bit"
> 
> Here "for" (compared to "on") is to signify it's controlled by target instead 
> of host spec.
Target feature is internal and mentioning it in release notes may be excessive 
(if we do, there are too many to enumerate ...). I think we can omit mentioning 
the target feature. Focusing on the behavior about the user-facing 
`-m[no-]unaligned-access` is more useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152671

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


[PATCH] D153469: [dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall added reviewers: mboehme, xazax.hun, gribozavr2.
sammccall added a comment.

Here we start to see benefits: Value becomes less reliant on inheritance, flow 
conditions are no longer Values that can uselessly bear properties, atomic 
variables are numbered in order of creation, we'll soon be able to have 
distinct BoolValues with the same formula (but different properties).

I think I could separate the Value/Arena changes from the ones to use 
Atom/Formula in DAContext if you think it's useful (the drawback is some risk 
of ending up in merge hell...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153469

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


[PATCH] D152946: [C++20][Modules] Implement P2615R1 revised export diagnostics.

2023-06-21 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:835
+
+  //  C++20 [module.interface]p3:
   if (auto *ND = dyn_cast(D)) {

Can you please add the quoted text for clarity. This is especially helpful if 
the text changes in the future and we are left trying to read the tea leaves to 
determine motivation. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152946

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


[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall added reviewers: gribozavr2, xazax.hun, mboehme.
sammccall added a comment.

Splitting out Formula from BoolValue is a pretty big change... I've tried to 
keep this patch a manageable size by really just using Formula for interacting 
with the SAT solver and some printing, but this means we're maintaining 
BoolValues and Formulas in parallel for now. Best to take a look at Formula.h, 
then look ahead to D153469  to see how we can 
eliminate the BoolValue subclasses, stop using Values as flow conditions in 
DataflowAnalysisContext, etc.

Would be very happy to hear high-level feedback/concerns first.
(I do know there's a bunch of places where words in comments/variable names 
should likely change - I thought making all these up front may make this very 
noisy to review)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153366

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


[PATCH] D151720: [clang][ExprConstant] Fix display of syntactically-invalid note for member function calls

2023-06-21 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:983
   EnableNewConstInterp(C.getLangOpts().EnableNewConstInterp),
-  BottomFrame(*this, SourceLocation(), nullptr, nullptr, CallRef()),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr,
+  /*CallExpr=*/nullptr, CallRef()),

Let's fix the missing one while here.



Comment at: clang/lib/AST/ExprConstant.cpp:1927
+  const Expr *Object = MCE->getImplicitObjectArgument();
+  Object->printPretty(Out, nullptr, Info.Ctx.getPrintingPolicy(), 0);
+  if (Object->getType()->isPointerType())





Comment at: clang/lib/AST/ExprConstant.cpp:1934-1935
+   dyn_cast_if_present(CallExpr)) {
+  OCE->getArg(0)->printPretty(Out, nullptr, Info.Ctx.getPrintingPolicy(),
+  0);
+  Out << ".";





Comment at: clang/lib/AST/ExprConstant.cpp:16289
   // Fabricate a call stack frame to give the arguments a plausible cover 
story.
-  CallStackFrame Frame(Info, SourceLocation(), FD, /*This*/ nullptr, 
CallRef());
+  CallStackFrame Frame(Info, SourceLocation(), FD, /*This*/ nullptr,
+   /*CallExpr=*/nullptr, CallRef());




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

https://reviews.llvm.org/D151720

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


[PATCH] D152671: [doc][LoongArch] Add missed release note about `ual` feature addition

2023-06-21 Thread WÁNG Xuěruì via Phabricator via cfe-commits
xen0n added a comment.

Thanks for the catch!




Comment at: llvm/docs/ReleaseNotes.rst:163
 * The ``lp64s`` ABI is supported now and has been tested on Rust bare-matal 
target.
+* An target feature ``ual`` is introduced to allow unaligned memory accesses 
and
+  this feature is enabled by default on generic 64bit processors.

nit: "A" (the following sound is not a vowel)



Comment at: llvm/docs/ReleaseNotes.rst:164
+* An target feature ``ual`` is introduced to allow unaligned memory accesses 
and
+  this feature is enabled by default on generic 64bit processors.
 

nit: "for generic 64-bit"

Here "for" (compared to "on") is to signify it's controlled by target instead 
of host spec.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152671

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


[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall added reviewers: ymandel, xazax.hun.
sammccall added a comment.

The idea with the change to Environment::join() is that the current signature 
forces the base environment to be mutable, but this sometimes forces the caller 
to make a copy, and the implementation actually copies everything anyway...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153493

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


[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 533463.
sammccall added a comment.

refactor, restore comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153493

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -179,6 +179,56 @@
   llvm::ArrayRef> BlockStates;
 };
 
+// Builds a joined TypeErasedDataflowAnalysisState from 0 or more sources,
+// each of which may be an owned copy or an immutable reference.
+// Avoids unneccesary copies of the environment.
+class JoinedStateBuilder {
+  AnalysisContext 
+  std::optional OwnedState;
+  const TypeErasedDataflowAnalysisState *CurrentState = nullptr;
+
+public:
+  JoinedStateBuilder(AnalysisContext ) : AC(AC) {}
+
+  void addOwned(TypeErasedDataflowAnalysisState State) {
+if (!CurrentState) {
+  OwnedState = std::move(State);
+  CurrentState = &*OwnedState;
+} else if (!OwnedState) {
+  OwnedState.emplace(std::move(CurrentState->Lattice),
+ CurrentState->Env.join(State.Env, AC.Analysis));
+  AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+} else {
+  OwnedState->Env = CurrentState->Env.join(State.Env, AC.Analysis);
+  AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+}
+  }
+  void addUnowned(const TypeErasedDataflowAnalysisState ) {
+if (!CurrentState) {
+  CurrentState = 
+} else if (!OwnedState) {
+  OwnedState.emplace(CurrentState->Lattice,
+ CurrentState->Env.join(State.Env, AC.Analysis));
+  AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+} else {
+  OwnedState->Env = CurrentState->Env.join(State.Env, AC.Analysis);
+  AC.Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+}
+  }
+  TypeErasedDataflowAnalysisState take() && {
+if (!OwnedState) {
+  if (CurrentState)
+OwnedState = *CurrentState;
+  else
+// FIXME: Consider passing `Block` to Analysis.typeErasedInitialElement
+// to enable building analyses like computation of dominators that
+// initialize the state of each basic block differently.
+OwnedState.emplace(AC.Analysis.typeErasedInitialElement(), AC.InitEnv);
+}
+return std::move(*OwnedState);
+  }
+};
+
 } // namespace
 
 /// Computes the input state for a given basic block by joining the output
@@ -225,9 +275,7 @@
 }
   }
 
-  std::optional MaybeState;
-
-  auto  = AC.Analysis;
+  JoinedStateBuilder Builder(AC);
   for (const CFGBlock *Pred : Preds) {
 // Skip if the `Block` is unreachable or control flow cannot get past it.
 if (!Pred || Pred->hasNoReturnElement())
@@ -240,36 +288,30 @@
 if (!MaybePredState)
   continue;
 
-TypeErasedDataflowAnalysisState PredState = *MaybePredState;
-if (Analysis.builtinOptions()) {
+if (AC.Analysis.builtinOptions()) {
   if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+// We have a terminator: we need to mutate an environment to describe
+// when the terminator is taken. Copy now.
+TypeErasedDataflowAnalysisState Copy = *MaybePredState;
+
 const StmtToEnvMap StmtToEnv(AC.CFCtx, AC.BlockStates);
 auto [Cond, CondValue] =
-TerminatorVisitor(StmtToEnv, PredState.Env,
+TerminatorVisitor(StmtToEnv, Copy.Env,
   blockIndexInPredecessor(*Pred, Block))
 .Visit(PredTerminatorStmt);
 if (Cond != nullptr)
   // FIXME: Call transferBranchTypeErased even if BuiltinTransferOpts
   // are not set.
-  Analysis.transferBranchTypeErased(CondValue, Cond, PredState.Lattice,
-PredState.Env);
+  AC.Analysis.transferBranchTypeErased(CondValue, Cond, Copy.Lattice,
+   Copy.Env);
+Builder.addOwned(std::move(Copy));
+continue;
   }
 }
-
-if (MaybeState) {
-  Analysis.joinTypeErased(MaybeState->Lattice, PredState.Lattice);
-  MaybeState->Env.join(PredState.Env, Analysis);
-} else {
-  MaybeState = std::move(PredState);
-}
-  }
-  if (!MaybeState) {
-// FIXME: Consider passing `Block` to `Analysis.typeErasedInitialElement()`
-// to enable building analyses like computation of dominators that
-// initialize the state of each basic block differently.
-MaybeState.emplace(Analysis.typeErasedInitialElement(), AC.InitEnv);
+

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is clunky but greatly improves debugging of flow conditions - each
copy adds more indirections in the form of flow condition tokens.

(LatticeEffect presumably once did something here, but it's now both
unused and untested.)

For the exit flow condition of:

  void target(base::Optional opt) {
if (opt.value_or(nullptr) != nullptr) {
  opt.value();
} else {
  opt.value(); // unsafe
}
  }

Before:

  (B0:1 = V15)
  (B1:1 = V8)
  (B2:1 = V10)
  (B3:1 = (V4 & (!V7 => V6)))
  (V10 = (B3:1 & !V7))
  (V12 = B1:1)
  (V13 = B2:1)
  (V15 = (V12 | V13))
  (V3 = V2)
  (V4 = V3)
  (V8 = (B3:1 & !!V7))
  B0:1
  V2

After D153491 :

  (B0:1 = (V9 | V10))
  (B1:1 = (B3:1 & !!V6))
  (B2:1 = (B3:1 & !V6))
  (B3:1 = (V3 & (!V6 => V5)))
  (V10 = B2:1)
  (V3 = V2)
  (V9 = B1:1)
  B0:1
  V2

After this patch, we can finally see the relations between the flow
conditions directly:

  (B0:1 = (B2:1 | B1:1))
  (B1:1 = (B3:1 & !!V6))
  (B2:1 = (B3:1 & !V6))
  (B3:1 = (V3 & (!V6 => V5)))
  (V3 = V2)
  B0:1
  V2

(I believe V2 is the FC for the InitEnv, and V3 is introduced when
computing the input state for B3  - not sure how 
to eliminate it)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153493

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -225,9 +225,45 @@
 }
   }
 
-  std::optional MaybeState;
+  std::optional OwnedState;
+  const TypeErasedDataflowAnalysisState *CurrentState = nullptr;
 
   auto  = AC.Analysis;
+  auto AddOwned = [&](TypeErasedDataflowAnalysisState State) {
+if (!CurrentState) {
+  OwnedState = std::move(State);
+  CurrentState = &*OwnedState;
+} else if (!OwnedState) {
+  OwnedState.emplace(std::move(CurrentState->Lattice),
+ CurrentState->Env.join(State.Env, Analysis));
+  Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+} else {
+  OwnedState->Env = CurrentState->Env.join(State.Env, Analysis);
+  Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+}
+  };
+  auto AddUnowned = [&](const TypeErasedDataflowAnalysisState ) {
+if (!CurrentState) {
+  CurrentState = 
+} else if (!OwnedState) {
+  OwnedState.emplace(CurrentState->Lattice,
+ CurrentState->Env.join(State.Env, Analysis));
+  Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+} else {
+  OwnedState->Env = CurrentState->Env.join(State.Env, Analysis);
+  Analysis.joinTypeErased(OwnedState->Lattice, State.Lattice);
+}
+  };
+  auto Finish = [&] {
+if (!OwnedState) {
+  if (CurrentState)
+OwnedState = *CurrentState;
+  else
+OwnedState.emplace(Analysis.typeErasedInitialElement(), AC.InitEnv);
+}
+return std::move(*OwnedState);
+  };
+
   for (const CFGBlock *Pred : Preds) {
 // Skip if the `Block` is unreachable or control flow cannot get past it.
 if (!Pred || Pred->hasNoReturnElement())
@@ -240,36 +276,30 @@
 if (!MaybePredState)
   continue;
 
-TypeErasedDataflowAnalysisState PredState = *MaybePredState;
 if (Analysis.builtinOptions()) {
   if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) {
+// We have a terminator: we need to mutate an environment to describe
+// when the terminator is taken. Copy now.
+TypeErasedDataflowAnalysisState Copy = *MaybePredState;
+
 const StmtToEnvMap StmtToEnv(AC.CFCtx, AC.BlockStates);
 auto [Cond, CondValue] =
-TerminatorVisitor(StmtToEnv, PredState.Env,
+TerminatorVisitor(StmtToEnv, Copy.Env,
   blockIndexInPredecessor(*Pred, Block))
 .Visit(PredTerminatorStmt);
 if (Cond != nullptr)
   // FIXME: Call transferBranchTypeErased even if BuiltinTransferOpts
   // are not set.
-  Analysis.transferBranchTypeErased(CondValue, Cond, PredState.Lattice,
-PredState.Env);
+  Analysis.transferBranchTypeErased(CondValue, Cond, Copy.Lattice,
+Copy.Env);
+AddOwned(std::move(Copy));
+continue;
   }
 }
-
-if (MaybeState) {
-  

[PATCH] D148697: [clang-tidy] Add more checks for functions which should be noexcept

2023-06-21 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp:273
+return Lexer::findLocationAfterToken(NoexceptLoc, tok::r_paren, SM,
+ LangOpts, true);
+

To conform with 
[burprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html).

Apologies for late review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148697

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


[PATCH] D152796: [clang][Sema] Fix diagnostic message for unused constant varialbe templates

2023-06-21 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1385
+  Diag(DiagD->getLocation(), diag::warn_unused_template)
+  << /*variable*/ 1 << DiagD;
 } else if (DiagD->getType().isConstQualified()) {

Sorry for late review but we should conform to 
[bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)
 for these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152796

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


[PATCH] D152788: [Clang] Show type in enum out of range diagnostic

2023-06-21 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment.

Thank you for this improvement!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152788

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


[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-06-21 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a reviewer: clang-language-wg.
shafik added a comment.

Adding clang-language-wg for more visibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965

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


[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

2023-06-21 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11226
+def err_module_private_use : Error<
+  "%select{declaration|definition|default argument|"
+  "explicit specialization|partial specialization}0 of %1 is private to module 
"

I only see `declaration` and `definition` cases covered for this diagnostic in 
the tests.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11230
+def err_module_internal_use : Error<
+  "%select{declaration|definition|default argument|"
+  "explicit specialization|partial specialization}0 of %1 is internal to "

I only see the `declaration` case covered for this diagnostic. Please add tests 
to cover the other cases.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11234
+def note_suggest_export : Note<
+  "export the %select{declaration|definition|default argument|"
+  "explicit specialization|partial specialization}0 to make it available">;

I only see the `declaration` and `definition` case covered in the tests we 
should cover all possible diagnostics. Especially in new code, bugs often come 
about on code we don't cover.



Comment at: clang/lib/Sema/SemaLookup.cpp:4520
+  for (/**/; DI != DE; ++DI) {
+if (!LookupResult::isVisible(SemaRef, *DI, /*ForTypo*/ true))
   break;

Fix to conform w/ 
[bugprone-argument-comment](https://clang.llvm.org/extra/clang-tidy/checks/bugprone/argument-comment.html)



Comment at: clang/lib/Sema/SemaLookup.cpp:4533
   for (/**/; DI != DE; ++DI) {
-if (LookupResult::isVisible(SemaRef, *DI)) {
+if (LookupResult::isVisible(SemaRef, *DI, /*ForTypo*/ true)) {
   if (!AnyVisibleDecls) {





Comment at: clang/lib/Sema/SemaLookup.cpp:4619
   // names that exactly match.
-  if (!LookupResult::isVisible(SemaRef, ND) && Name != Typo)
+  if (!LookupResult::isVisible(SemaRef, ND, /*ForTypo*/ true) && Name != Typo)
 return;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965

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


[PATCH] D153491: [dataflow] Avoid copying environment

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This appears to be just an accidental copy rather than move from a scratch
variable.
As well as doing redundant work, these copies introduce extra SAT variables
which make debugging harder (each Enviroment has a unique FC token).

Example flow condition before:

  (B0:1 = V15)
  (B1:1 = V8)
  (B2:1 = V10)
  (B3:1 = (V4 & (!V7 => V6)))
  (V10 = (B3:1 & !V7))
  (V12 = B1:1)
  (V13 = B2:1)
  (V15 = (V12 | V13))
  (V3 = V2)
  (V4 = V3)
  (V8 = (B3:1 & !!V7))
  B0:1
  V2

after:

  (B0:1 = (V9 | V10))
  (B1:1 = (B3:1 & !!V6))
  (B2:1 = (B3:1 & !V6))
  (B3:1 = (V3 & (!V6 => V5)))
  (V10 = B2:1)
  (V3 = V2)
  (V9 = B1:1)
  B0:1
  V2

(with labelling from D153488 )

There are also some more copies that can be avoided here (when multiple blocks
without terminating statements are joined), but they're less trivial, so I'll
put those in another patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153491

Files:
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -269,7 +269,7 @@
 // initialize the state of each basic block differently.
 MaybeState.emplace(Analysis.typeErasedInitialElement(), AC.InitEnv);
   }
-  return *MaybeState;
+  return std::move(*MaybeState);
 }
 
 /// Built-in transfer function for `CFGStmt`.


Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -269,7 +269,7 @@
 // initialize the state of each basic block differently.
 MaybeState.emplace(Analysis.typeErasedInitialElement(), AC.InitEnv);
   }
-  return *MaybeState;
+  return std::move(*MaybeState);
 }
 
 /// Built-in transfer function for `CFGStmt`.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

2023-06-21 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm closed this revision.
arsenm added a comment.

1717c18664d5880f78db85eb0075a2c1379df2d9 



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

https://reviews.llvm.org/D142823

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


[PATCH] D138397: HIP: Directly call ldexp builtins

2023-06-21 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm closed this revision.
arsenm added a comment.

c448e1dabeeb066c41d893fc8e7a3d8bde2056c8 



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

https://reviews.llvm.org/D138397

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


[clang] c448e1d - HIP: Directly call ldexp builtins

2023-06-21 Thread Matt Arsenault via cfe-commits

Author: Matt Arsenault
Date: 2023-06-21T21:34:42-04:00
New Revision: c448e1dabeeb066c41d893fc8e7a3d8bde2056c8

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

LOG: HIP: Directly call ldexp builtins

Added: 


Modified: 
clang/lib/Headers/__clang_hip_math.h
clang/test/Headers/__clang_hip_math.hip

Removed: 




diff  --git a/clang/lib/Headers/__clang_hip_math.h 
b/clang/lib/Headers/__clang_hip_math.h
index 2b33efcd8317a..850dbbef0f4e3 100644
--- a/clang/lib/Headers/__clang_hip_math.h
+++ b/clang/lib/Headers/__clang_hip_math.h
@@ -304,7 +304,7 @@ float jnf(int __n, float __x) { // TODO: we could use Ahmes 
multiplication
 }
 
 __DEVICE__
-float ldexpf(float __x, int __e) { return __ocml_ldexp_f32(__x, __e); }
+float ldexpf(float __x, int __e) { return __builtin_amdgcn_ldexpf(__x, __e); }
 
 __DEVICE__
 float lgammaf(float __x) { return __ocml_lgamma_f32(__x); }
@@ -468,12 +468,12 @@ float rsqrtf(float __x) { return __ocml_rsqrt_f32(__x); }
 
 __DEVICE__
 float scalblnf(float __x, long int __n) {
-  return (__n < INT_MAX) ? __ocml_scalbn_f32(__x, __n)
+  return (__n < INT_MAX) ? __builtin_amdgcn_ldexpf(__x, __n)
  : __ocml_scalb_f32(__x, __n);
 }
 
 __DEVICE__
-float scalbnf(float __x, int __n) { return __ocml_scalbn_f32(__x, __n); }
+float scalbnf(float __x, int __n) { return __builtin_amdgcn_ldexpf(__x, __n); }
 
 __DEVICE__
 __RETURN_TYPE __signbitf(float __x) { return __builtin_signbitf(__x); }
@@ -853,7 +853,7 @@ double jn(int __n, double __x) { // TODO: we could use 
Ahmes multiplication
 }
 
 __DEVICE__
-double ldexp(double __x, int __e) { return __ocml_ldexp_f64(__x, __e); }
+double ldexp(double __x, int __e) { return __builtin_amdgcn_ldexp(__x, __e); }
 
 __DEVICE__
 double lgamma(double __x) { return __ocml_lgamma_f64(__x); }
@@ -1025,11 +1025,11 @@ double rsqrt(double __x) { return 
__ocml_rsqrt_f64(__x); }
 
 __DEVICE__
 double scalbln(double __x, long int __n) {
-  return (__n < INT_MAX) ? __ocml_scalbn_f64(__x, __n)
+  return (__n < INT_MAX) ? __builtin_amdgcn_ldexp(__x, __n)
  : __ocml_scalb_f64(__x, __n);
 }
 __DEVICE__
-double scalbn(double __x, int __n) { return __ocml_scalbn_f64(__x, __n); }
+double scalbn(double __x, int __n) { return __builtin_amdgcn_ldexp(__x, __n); }
 
 __DEVICE__
 __RETURN_TYPE __signbit(double __x) { return __builtin_signbit(__x); }

diff  --git a/clang/test/Headers/__clang_hip_math.hip 
b/clang/test/Headers/__clang_hip_math.hip
index bb96aeaa1ab9b..80501d1d43e46 100644
--- a/clang/test/Headers/__clang_hip_math.hip
+++ b/clang/test/Headers/__clang_hip_math.hip
@@ -1439,13 +1439,13 @@ extern "C" __device__ double test_jn(int x, double y) {
 
 // DEFAULT-LABEL: @test_ldexpf(
 // DEFAULT-NEXT:  entry:
-// DEFAULT-NEXT:[[CALL_I:%.*]] = tail call contract float 
@__ocml_ldexp_f32(float noundef [[X:%.*]], i32 noundef [[Y:%.*]]) #[[ATTR14]]
-// DEFAULT-NEXT:ret float [[CALL_I]]
+// DEFAULT-NEXT:[[TMP0:%.*]] = tail call contract float 
@llvm.ldexp.f32.i32(float [[X:%.*]], i32 [[Y:%.*]])
+// DEFAULT-NEXT:ret float [[TMP0]]
 //
 // FINITEONLY-LABEL: @test_ldexpf(
 // FINITEONLY-NEXT:  entry:
-// FINITEONLY-NEXT:[[CALL_I:%.*]] = tail call nnan ninf contract 
nofpclass(nan inf) float @__ocml_ldexp_f32(float noundef nofpclass(nan inf) 
[[X:%.*]], i32 noundef [[Y:%.*]]) #[[ATTR14]]
-// FINITEONLY-NEXT:ret float [[CALL_I]]
+// FINITEONLY-NEXT:[[TMP0:%.*]] = tail call nnan ninf contract float 
@llvm.ldexp.f32.i32(float [[X:%.*]], i32 [[Y:%.*]])
+// FINITEONLY-NEXT:ret float [[TMP0]]
 //
 extern "C" __device__ float test_ldexpf(float x, int y) {
   return ldexpf(x, y);
@@ -1453,13 +1453,13 @@ extern "C" __device__ float test_ldexpf(float x, int y) 
{
 
 // DEFAULT-LABEL: @test_ldexp(
 // DEFAULT-NEXT:  entry:
-// DEFAULT-NEXT:[[CALL_I:%.*]] = tail call contract double 
@__ocml_ldexp_f64(double noundef [[X:%.*]], i32 noundef [[Y:%.*]]) #[[ATTR14]]
-// DEFAULT-NEXT:ret double [[CALL_I]]
+// DEFAULT-NEXT:[[TMP0:%.*]] = tail call contract double 
@llvm.ldexp.f64.i32(double [[X:%.*]], i32 [[Y:%.*]])
+// DEFAULT-NEXT:ret double [[TMP0]]
 //
 // FINITEONLY-LABEL: @test_ldexp(
 // FINITEONLY-NEXT:  entry:
-// FINITEONLY-NEXT:[[CALL_I:%.*]] = tail call nnan ninf contract 
nofpclass(nan inf) double @__ocml_ldexp_f64(double noundef nofpclass(nan inf) 
[[X:%.*]], i32 noundef [[Y:%.*]]) #[[ATTR14]]
-// FINITEONLY-NEXT:ret double [[CALL_I]]
+// FINITEONLY-NEXT:[[TMP0:%.*]] = tail call nnan ninf contract double 
@llvm.ldexp.f64.i32(double [[X:%.*]], i32 [[Y:%.*]])
+// FINITEONLY-NEXT:ret double [[TMP0]]
 //
 extern "C" __device__ double test_ldexp(double x, int y) {
   return ldexp(x, y);
@@ -2688,13 +2688,13 @@ extern "C" __device__ double 

[PATCH] D153488: [dataflow] HTMLLogger: meaningful names for flow condition variables

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Instead of meaningless "V7", use the name "B3 :1" 
for the flow condition
of the first analysis of basic block 3.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153488

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp

Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -154,6 +154,8 @@
   std::vector> Iters;
   // Number of times each CFG block has been seen.
   llvm::DenseMap BlockIters;
+  // Labels assigned to SAT atoms.
+  llvm::DenseMap AtomNames;
   // The messages logged in the current context but not yet written.
   std::string ContextLogs;
   // The number of elements we have visited within the current CFG block.
@@ -245,6 +247,9 @@
   void recordState(TypeErasedDataflowAnalysisState ) override {
 unsigned Block = Iters.back().first->getBlockID();
 unsigned Iter = Iters.back().second;
+AtomNames.try_emplace(State.Env.getFlowConditionToken(),
+  iterID(Block, Iter));
+
 JOS->attributeObject(elementIterID(Block, Iter, ElementIndex), [&] {
   JOS->attribute("block", blockID(Block));
   JOS->attribute("iter", Iter);
@@ -266,7 +271,17 @@
   {
 std::string BuiltinLattice;
 llvm::raw_string_ostream BuiltinLatticeS(BuiltinLattice);
-State.Env.dump(BuiltinLatticeS);
+State.Env.dump(BuiltinLatticeS,
+   [&](llvm::raw_ostream , const Formula ) {
+ if (F.kind() == Formula::AtomRef) {
+   if (auto It = AtomNames.find(F.atom());
+   It != AtomNames.end()) {
+ OS << It->second;
+ return true;
+   }
+ }
+ return false;
+   });
 JOS->attribute("builtinLattice", BuiltinLattice);
   }
 });
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -898,7 +898,9 @@
   return flowConditionImplies(Val.formula());
 }
 
-void Environment::dump(raw_ostream ) const {
+void Environment::dump(
+raw_ostream ,
+llvm::function_ref Delegate) const {
   // FIXME: add printing for remaining fields and allow caller to decide what
   // fields are printed.
   OS << "DeclToLoc:\n";
@@ -914,8 +916,8 @@
 OS << "  [" << L << ", " << V << ": " << *V << "]\n";
   }
 
-  OS << "FlowConditionToken:\n";
-  DACtx->dumpFlowCondition(FlowConditionToken, OS);
+  OS << "Flow condition:\n";
+  DACtx->dumpFlowCondition(FlowConditionToken, OS, Delegate);
 }
 
 void Environment::dump() const {
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -189,8 +189,9 @@
   }
 }
 
-void DataflowAnalysisContext::dumpFlowCondition(Atom Token,
-llvm::raw_ostream ) {
+void DataflowAnalysisContext::dumpFlowCondition(
+Atom Token, llvm::raw_ostream ,
+llvm::function_ref OuterDelegate) {
   llvm::DenseSet Constraints = {().makeAtomRef(Token)};
   llvm::DenseSet VisitedTokens;
   addTransitiveFlowConditionConstraints(Token, Constraints, VisitedTokens);
@@ -199,6 +200,8 @@
   auto  = arena().makeLiteral(true);
   auto  = arena().makeLiteral(false);
   auto Delegate = [&](llvm::raw_ostream , const Formula ) {
+if (OuterDelegate(OS, F))
+  return true;
 if ( == ) {
   OS << "True";
   return true;
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -548,7 +548,9 @@
   Arena () const { return DACtx->arena(); }
 
   LLVM_DUMP_METHOD void dump() const;
-  LLVM_DUMP_METHOD void dump(raw_ostream ) const;
+  LLVM_DUMP_METHOD void
+  dump(raw_ostream ,
+   llvm::function_ref = 

[PATCH] D153228: [clang-format] Fixed bad performance with enabled qualifier fixer.

2023-06-21 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/lib/Format/Format.cpp:3476-3477
+
+  // Regarding the 16: Note that multiple passes are added in
+  // addQualifierAlignmentFixerPasses().
+  SmallVector Passes;

This comment is unnecessary and too verbose IMO. I suggest that it be deleted.



Comment at: clang/lib/Format/Format.cpp:3571-3585
+  // Don't make replacements that replace nothing. This can affect e.g. the
+  // output of clang-format with the --output-replacements-xml option.
+  tooling::Replacements NonNoOpFixes;
+  for (const tooling::Replacement  : Fixes) {
+StringRef OriginalCode = Code.substr(Fix.getOffset(), Fix.getLength());
+if (!OriginalCode.equals(Fix.getReplacementText())) {
+  auto Err = NonNoOpFixes.add(Fix);

You only need this for the `QualifierAlignment` passes as others (e.g. 
`IntegerLiteralSeparatorFixer`) already skip no-op replacements.



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:29
+void addQualifierAlignmentFixerPasses(const FormatStyle ,
+  SmallVector ) {
   std::vector LeftOrder;

Ditto.



Comment at: clang/lib/Format/QualifierAlignmentFixer.h:29
+void addQualifierAlignmentFixerPasses(const FormatStyle ,
+  SmallVector );
 

See https://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153228

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


[PATCH] D153485: [dataflow] use true/false literals in formulas, rather than variables

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

And simplify formulas containing true/false
It's unclear to me how useful this is, it does make formulas more
conveniently self-contained now (we can usefully print them without
carrying around the "true/false" labels)

(while here, simplify !!X to X, too)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153485

Files:
  clang/include/clang/Analysis/FlowSensitive/Arena.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/Formula.h
  clang/lib/Analysis/FlowSensitive/Arena.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/Formula.cpp
  clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
  clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
  clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2853,14 +2853,14 @@
 ASSERT_THAT(FooDecl, NotNull());
 
 const auto *FooVal =
-dyn_cast_or_null(Env.getValue(*FooDecl));
+dyn_cast_or_null(Env.getValue(*FooDecl));
 ASSERT_THAT(FooVal, NotNull());
 
 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
 ASSERT_THAT(BarDecl, NotNull());
 
 const auto *BarVal =
-dyn_cast_or_null(Env.getValue(*BarDecl));
+dyn_cast_or_null(Env.getValue(*BarDecl));
 ASSERT_THAT(BarVal, NotNull());
 
 EXPECT_EQ(FooVal, (true));
@@ -3038,7 +3038,7 @@
 ASSERT_THAT(FooDecl, NotNull());
 
 const auto *FooVal =
-dyn_cast_or_null(Env.getValue(*FooDecl));
+dyn_cast_or_null(Env.getValue(*FooDecl));
 ASSERT_THAT(FooVal, NotNull());
 
 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -418,6 +418,11 @@
 return ::create(A, Formula::AtomRef, {}, NextAtom++);
   }
 
+  // Creates a reference to a literal boolean value.
+  const Formula *literal(bool B) {
+return ::create(A, Formula::Literal, {}, B);
+  }
+
   // Creates a boolean conjunction value.
   const Formula *conj(const Formula *LeftSubVal, const Formula *RightSubVal) {
 return make(Formula::And, {LeftSubVal, RightSubVal});
Index: clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DebugSupportTest.cpp
@@ -32,6 +32,12 @@
   EXPECT_THAT(llvm::to_string(*B), StrEq(Expected));
 }
 
+TEST(BoolValueDebugStringTest, Literal) {
+  ConstraintContext Ctx;
+  EXPECT_EQ("true", llvm::to_string(*Ctx.literal(true)));
+  EXPECT_EQ("false", llvm::to_string(*Ctx.literal(false)));
+}
+
 TEST(BoolValueDebugStringTest, Negation) {
   ConstraintContext Ctx;
   auto B = Ctx.neg(Ctx.atom());
@@ -95,22 +101,22 @@
 
 TEST(BoolValueDebugStringTest, ComplexBooleanWithSomeNames) {
   ConstraintContext Ctx;
-  auto True = Ctx.atom();
-  auto False = Ctx.atom();
+  auto X = Ctx.atom();
+  auto Y = Ctx.atom();
   auto Delegate = [&](llvm::raw_ostream , const Formula ) {
-if ( == True) {
-  OS << "True";
+if ( == X) {
+  OS << "X";
   return true;
 }
-if ( == False) {
-  OS << "False";
+if ( == Y) {
+  OS << "Y";
   return true;
 }
 return false;
   };
-  auto B = Ctx.disj(Ctx.conj(False, Ctx.atom()), Ctx.disj(True, Ctx.atom()));
+  auto B = Ctx.disj(Ctx.conj(Y, Ctx.atom()), Ctx.disj(X, Ctx.atom()));
 
-  auto Expected = R"(((False & V2) | (True | V3)))";
+  auto Expected = R"(((Y & V2) | (X | V3)))";
   std::string Actual;
   llvm::raw_string_ostream OS(Actual);
   B->print(OS, Delegate);
Index: clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
@@ -31,12 +31,6 @@
   EXPECT_NE(, );
 }
 
-TEST_F(ArenaTest, GetOrCreateConjunctionReturnsSameExprGivenSameArgs) {
-  auto  = A.makeAtomRef(A.makeAtom());
-  auto  = A.makeAnd(X, X);
-  EXPECT_EQ(, );
-}
-
 TEST_F(ArenaTest, 

[PATCH] D153379: Remove -flang-experimental-exec flag

2023-06-21 Thread Katherine Rasmussen via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0867d4157328: [flang] dont require 
-flang-experimental-exec flag anymore (authored by everythingfunctional, 
committed by ktras).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153379

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  flang/docs/FlangDriver.md
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/linker-flags.f90

Index: flang/test/Driver/linker-flags.f90
===
--- flang/test/Driver/linker-flags.f90
+++ flang/test/Driver/linker-flags.f90
@@ -2,15 +2,15 @@
 ! invocation. These libraries are added on top of other standard runtime
 ! libraries that the Clang driver will include.
 
-! RUN: %flang -### -flang-experimental-exec -target ppc64le-linux-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,GNU
-! RUN: %flang -### -flang-experimental-exec -target aarch64-apple-darwin %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,DARWIN
-! RUN: %flang -### -flang-experimental-exec -target x86_64-windows-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MINGW
+! RUN: %flang -### -target ppc64le-linux-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,GNU
+! RUN: %flang -### -target aarch64-apple-darwin %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,DARWIN
+! RUN: %flang -### -target x86_64-windows-gnu %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MINGW
 
 ! NOTE: Clang's driver library, clangDriver, usually adds 'libcmt' and
 !   'oldnames' on Windows, but they are not needed when compiling
 !   Fortran code and they might bring in additional dependencies.
 !   Make sure they're not added.
-! RUN: %flang -### -flang-experimental-exec -target aarch64-windows-msvc %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MSVC --implicit-check-not libcmt --implicit-check-not oldnames
+! RUN: %flang -### -target aarch64-windows-msvc %S/Inputs/hello.f90 2>&1 | FileCheck %s --check-prefixes=CHECK,MSVC --implicit-check-not libcmt --implicit-check-not oldnames
 
 ! Compiler invocation to generate the object file
 ! CHECK-LABEL: {{.*}} "-emit-obj"
Index: flang/test/Driver/driver-help-hidden.f90
===
--- flang/test/Driver/driver-help-hidden.f90
+++ flang/test/Driver/driver-help-hidden.f90
@@ -39,8 +39,6 @@
 ! CHECK-NEXT: -finput-charset= Specify the default character set for source files
 ! CHECK-NEXT: -fintrinsic-modules-path 
 ! CHECK-NEXT:Specify where to find the compiled intrinsic modules
-! CHECK-NEXT: -flang-experimental-exec
-! CHECK-NEXT:Enable support for generating executables (experimental)
 ! CHECK-NEXT: -flang-experimental-hlfir
 ! CHECK-NEXT:Use HLFIR lowering (experimental)
 ! CHECK-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type in size-related intrinsics
Index: flang/docs/FlangDriver.md
===
--- flang/docs/FlangDriver.md
+++ flang/docs/FlangDriver.md
@@ -149,11 +149,6 @@
 +- 3: backend, {2}, assembler
 4: assembler, {3}, object
 ```
-Note that currently Flang does not support code-generation and `flang-new` will
-fail during the second step above with the following error:
-```bash
-error: code-generation is not available yet
-```
 The other phases are printed nonetheless when using `-ccc-print-phases`, as
 that reflects what `clangDriver`, the library, will try to create and run.
 
@@ -330,16 +325,13 @@
 supported Fortran compiler. You can configure your CMake projects to use
 `flang-new` as follows:
 ```bash
-cmake -DCMAKE_Fortran_FLAGS="-flang-experimental-exec" -DCMAKE_Fortran_COMPILER= 
+cmake -DCMAKE_Fortran_COMPILER= 
 ```
 You should see the following in the output:
 ```
 -- The Fortran compiler identification is LLVMFlang 
 ```
-where `` corresponds to the LLVM Flang version. Note that while
-generating executables remains experimental, you will need to inform CMake to
-use the `-flang-experimental-exec` flag when invoking `flang-new` as in the
-example above.
+where `` corresponds to the LLVM Flang version.
 
 # Testing
 In LIT, we define two variables that you can use to invoke Flang's drivers:
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -935,12 +935,6 @@
 void tools::addFortranRuntimeLibraryPath(const ToolChain ,
  const llvm::opt::ArgList ,
  ArgStringList ) {
-  // NOTE: Generating executables by Flang is considered an 

[clang] 0867d41 - [flang] don't require -flang-experimental-exec flag anymore

2023-06-21 Thread Katherine Rasmussen via cfe-commits

Author: Brad Richardson
Date: 2023-06-21T16:47:52-07:00
New Revision: 0867d4157328169c570f4d1e9c01806624035873

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

LOG: [flang] don't require -flang-experimental-exec flag anymore

Reviewed By: awarzynski, PeteSteinfeld

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/CommonArgs.cpp
flang/docs/FlangDriver.md
flang/test/Driver/driver-help-hidden.f90
flang/test/Driver/linker-flags.f90

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index cf002e772df02..69239e6cf296d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5279,10 +5279,6 @@ def fno_sycl : Flag<["-"], "fno-sycl">, 
Flags<[NoXarchOption, CoreOption]>,
 // FLangOption + NoXarchOption
 
//===--===//
 
-def flang_experimental_exec : Flag<["-"], "flang-experimental-exec">,
-  Flags<[FlangOption, FlangOnlyOption, NoXarchOption, HelpHidden]>,
-  HelpText<"Enable support for generating executables (experimental)">;
-
 def flang_experimental_hlfir : Flag<["-"], "flang-experimental-hlfir">,
   Flags<[FlangOption, FC1Option, FlangOnlyOption, NoXarchOption, HelpHidden]>,
   HelpText<"Use HLFIR lowering (experimental)">;

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 2d9392893bc36..4afe3cc1f7a6e 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -935,12 +935,6 @@ void tools::addFortranRuntimeLibs(const ToolChain ,
 void tools::addFortranRuntimeLibraryPath(const ToolChain ,
  const llvm::opt::ArgList ,
  ArgStringList ) {
-  // NOTE: Generating executables by Flang is considered an "experimental"
-  // feature and hence this is guarded with a command line option.
-  // TODO: Make this work unconditionally once Flang is mature enough.
-  if (!Args.hasArg(options::OPT_flang_experimental_exec))
-return;
-
   // Default to the /../lib directory. This works fine on the
   // platforms that we have tested so far. We will probably have to re-fine
   // this in the future. In particular, on some platforms, we may need to use

diff  --git a/flang/docs/FlangDriver.md b/flang/docs/FlangDriver.md
index 6c2a473820634..11e86289cd6f6 100644
--- a/flang/docs/FlangDriver.md
+++ b/flang/docs/FlangDriver.md
@@ -149,11 +149,6 @@ flang-new -ccc-print-phases -c file.f
 +- 3: backend, {2}, assembler
 4: assembler, {3}, object
 ```
-Note that currently Flang does not support code-generation and `flang-new` will
-fail during the second step above with the following error:
-```bash
-error: code-generation is not available yet
-```
 The other phases are printed nonetheless when using `-ccc-print-phases`, as
 that reflects what `clangDriver`, the library, will try to create and run.
 
@@ -330,16 +325,13 @@ As of 
[#7246](https://gitlab.kitware.com/cmake/cmake/-/merge_requests/7246)
 supported Fortran compiler. You can configure your CMake projects to use
 `flang-new` as follows:
 ```bash
-cmake -DCMAKE_Fortran_FLAGS="-flang-experimental-exec" 
-DCMAKE_Fortran_COMPILER= 
+cmake -DCMAKE_Fortran_COMPILER= 
 ```
 You should see the following in the output:
 ```
 -- The Fortran compiler identification is LLVMFlang 
 ```
-where `` corresponds to the LLVM Flang version. Note that while
-generating executables remains experimental, you will need to inform CMake to
-use the `-flang-experimental-exec` flag when invoking `flang-new` as in the
-example above.
+where `` corresponds to the LLVM Flang version.
 
 # Testing
 In LIT, we define two variables that you can use to invoke Flang's drivers:

diff  --git a/flang/test/Driver/driver-help-hidden.f90 
b/flang/test/Driver/driver-help-hidden.f90
index 002b30cfe283b..d8b1a03d4c0d9 100644
--- a/flang/test/Driver/driver-help-hidden.f90
+++ b/flang/test/Driver/driver-help-hidden.f90
@@ -39,8 +39,6 @@
 ! CHECK-NEXT: -finput-charset= Specify the default character set for 
source files
 ! CHECK-NEXT: -fintrinsic-modules-path 
 ! CHECK-NEXT:Specify where to find the compiled 
intrinsic modules
-! CHECK-NEXT: -flang-experimental-exec
-! CHECK-NEXT:Enable support for generating executables 
(experimental)
 ! CHECK-NEXT: -flang-experimental-hlfir
 ! CHECK-NEXT:Use HLFIR lowering (experimental)
 ! CHECK-NEXT: -flarge-sizes  Use INTEGER(KIND=8) for the result type 
in size-related intrinsics

diff  --git 

[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-06-21 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2311
   MarshallingInfoString>;
+defm fat_lto_objects : BoolFOption<"fat-lto-objects",
+  CodeGenOpts<"FatLTO">, DefaultFalse,

MaskRay wrote:
> We just need the pos flag for CC1Option, so use `OptInCC1FFlag`
Wouldn't that prevent me from setting `CodeGenOpts<"FatLTO">`? AFAICT 
`OptInCC1Flag` doesn't take a `CodeGenOpts` since it isn't an `OptionsFlag`.



Comment at: clang/test/CodeGen/embed-lto-fatlto.c:8
+/// Check that the ThinLTO metadata is only set false for full LTO
+// FULL: !{{[0-9]+}} = !{i32 1, !"ThinLTO", i32 0}
+// THIN-NOT: !{{[0-9]+}} = !{i32 1, !"ThinLTO", i32 0}

MaskRay wrote:
> 
Extremely useful to know. Not sure how I've missed that until now. Thanks!



Comment at: clang/test/Driver/fatlto-objects.c:7
+
+// RUN: %clang --target=x86_64-unknown-linux-gnu -ffat-lto-objects 
-fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
+// CHECK-CC-NOLTO: -cc1

MaskRay wrote:
> You may test that `-ffat-lto-objects` in the absence of `-flto=` gives a 
> `warning: argument unused during compilation: '-ffat-lto-objects'` warning
Since I've slightly refactored the driver code based on your comments, we 
shouldn't get that warning anymore. I've tested that instead, but we can also 
drop it, since I'm not sure how much value that adds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777

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


[PATCH] D146777: [clang] Preliminary fat-lto-object support

2023-06-21 Thread Paul Kirth via Phabricator via cfe-commits
paulkirth updated this revision to Diff 533433.
paulkirth marked 12 inline comments as done.
paulkirth added a comment.

Rebase and address comments

- Update & rename tests
- Fix typos
- Handle options


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CodeGen/embed-fat-lto-objects.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fat-lto-objects.c

Index: clang/test/Driver/fat-lto-objects.c
===
--- /dev/null
+++ clang/test/Driver/fat-lto-objects.c
@@ -0,0 +1,19 @@
+// RUN: %clang --target=x86_64-unknown-linux-gnu -flto -ffat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
+// CHECK-CC: -cc1
+// CHECK-CC-SAME: -emit-obj
+// CHECK-CC-SAME: -ffat-lto-objects
+
+// RUN: %clang --target=x86_64-unknown-linux-gnu -ffat-lto-objects -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
+// CHECK-CC-NOLTO: -cc1
+// CHECK-CC-NOLTO-SAME: -emit-obj
+// CHECK-CC-NOLTO-NOT: -ffat-lto-objects
+// CHECK-CC-NOLTO-NOT: warning: argument unused during compilation: '-ffat-lto-objects'
+
+/// We need to pass an additional flag to lld when linking w/ -flto -ffat-lto-objects
+/// But it should not be there when LTO is disabled w/ -fno-lto
+// RUN: %clang --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/basic_cross_linux_tree %s \
+// RUN:   -fuse-ld=lld -flto -ffat-lto-objects -### 2>&1 | FileCheck --check-prefix=LTO %s
+// RUN: %clang --target=x86_64-unknown-linux-gnu --sysroot=%S/Inputs/basic_cross_linux_tree %s \
+// RUN:   -fuse-ld=lld -fno-lto -ffat-lto-objects -### 2>&1 | FileCheck --check-prefix=NOLTO %s
+// LTO: "-fat-lto-objects"
+// NOLTO-NOT: "-fat-lto-objects"
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -425,7 +425,6 @@
 // CHECK-WARNING-DAG: optimization flag '-fwhole-program' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fcaller-saves' is not supported
 // CHECK-WARNING-DAG: optimization flag '-freorder-blocks' is not supported
-// CHECK-WARNING-DAG: optimization flag '-ffat-lto-objects' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fmerge-constants' is not supported
 // CHECK-WARNING-DAG: optimization flag '-finline-small-functions' is not supported
 // CHECK-WARNING-DAG: optimization flag '-ftree-dce' is not supported
Index: clang/test/CodeGen/embed-fat-lto-objects.c
===
--- /dev/null
+++ clang/test/CodeGen/embed-fat-lto-objects.c
@@ -0,0 +1,17 @@
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=full -ffat-lto-objects -fsplit-lto-unit -emit-llvm < %s  | FileCheck %s --check-prefixes=FULL,SPLIT
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s --check-prefixes=FULL,SPLIT
+
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=thin -fsplit-lto-unit -ffat-lto-objects -emit-llvm < %s  | FileCheck %s --check-prefixes=THIN,SPLIT
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -flto=thin -ffat-lto-objects -emit-llvm < %s  | FileCheck %s --check-prefixes=THIN,NOSPLIT
+
+/// Check that the ThinLTO metadata is only set false for full LTO.
+// FULL: ![[#]] = !{i32 1, !"ThinLTO", i32 0}
+// THIN-NOT: ![[#]] = !{i32 1, !"ThinLTO", i32 0}
+
+/// Be sure we enable split LTO unints correctly under -ffat-lto-objects.
+// SPLIT: ![[#]] = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+// NOSPLIT: ![[#]] = !{i32 1, !"EnableSplitLTOUnit", i32 0}
+
+int test(void) {
+  return 0xabcd;
+}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -619,6 +619,10 @@
 PluginName + Suffix,
 Plugin);
 CmdArgs.push_back(Args.MakeArgString(Twine(PluginPrefix) + Plugin));
+  } else {
+// For LLD we need to enable fat object support
+if (Args.hasArg(options::OPT_ffat_lto_objects))
+  CmdArgs.push_back("-fat-lto-objects");
   }
 
   const char *PluginOptPrefix = IsOSAIX ? "-bplugin_opt:" : "-plugin-opt=";
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7263,6 +7263,22 @@
   if (SplitLTOUnit)
 CmdArgs.push_back("-fsplit-lto-unit");
 
+  if (Arg *A = 

[PATCH] D153481: [clang] NFCI: Use `DirectoryEntryRef` in framework lookup

2023-06-21 Thread Fernando Salas via Phabricator via cfe-commits
frs513 created this revision.
Herald added a project: All.
frs513 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This removes one use of the deprecated `DirectoryEntry::getName()`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153481

Files:
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -690,7 +690,7 @@
   // If we found the header and are allowed to suggest a module, do so now.
   if (File && needModuleLookup(RequestingModule, SuggestedModule)) {
 // Find the framework in which this header occurs.
-StringRef FrameworkPath = File->getFileEntry().getDir()->getName();
+StringRef FrameworkPath = File->getDir().getName();
 bool FoundFramework = false;
 do {
   // Determine whether this directory exists.


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -690,7 +690,7 @@
   // If we found the header and are allowed to suggest a module, do so now.
   if (File && needModuleLookup(RequestingModule, SuggestedModule)) {
 // Find the framework in which this header occurs.
-StringRef FrameworkPath = File->getFileEntry().getDir()->getName();
+StringRef FrameworkPath = File->getDir().getName();
 bool FoundFramework = false;
 do {
   // Determine whether this directory exists.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D150744: [NFC][CLANG] Fix uninitialized scalar field issues

2023-06-21 Thread Sindhu Chittireddy via Phabricator via cfe-commits
schittir added a comment.

@tahonermann @shafik - is it ok to land this patch?


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

https://reviews.llvm.org/D150744

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


[PATCH] D153462: [Headers][doc] Add various arith/logical intrinsic descriptions to avx2intrin.h

2023-06-21 Thread Craig Topper via Phabricator via cfe-commits
craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D153462

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


[PATCH] D150489: [-Wunsafe-buffer-usage] Handle pointer initializations for grouping related variables

2023-06-21 Thread Rashmi Mudduluru 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 rGdb3dcedb9ced: [-Wunsafe-buffer-usage] Handle pointer 
initializations for grouping related… (authored by t-rasmud).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D150489?vs=531912=533425#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150489

Files:
  clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init-fixits.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc-fixits.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp
  clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp

Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp
@@ -39,13 +39,11 @@
   p = q;
 }
 
-
-// FIXME: Support initializations at declarations.
 void lhs_span_multi_assign() {
   int *a = new int[2];
   int *b = a;
   int *c = b;
-  int *d = c;  // expected-warning{{'d' is an unsafe pointer used for buffer access}} expected-note-re^change type of 'd' to 'std::span' to preserve bounds information$
+  int *d = c;  // expected-warning{{'d' is an unsafe pointer used for buffer access}} expected-note-re^change type of 'd' to 'std::span' to preserve bounds information, and change ('a', 'b', and 'c'|'a', 'c', and 'b'|'b', 'a', and 'c'|'b', 'c', and 'a'|'c', 'a', and 'b'|'c', 'b', and 'a') to 'std::span' to propagate bounds information between them$
   int tmp = d[2];  // expected-note{{used in buffer access here}}
 }
 
@@ -59,15 +57,15 @@
 
 void rhs_span1() {
   int *q = new int[12];
-  int *p = q;  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re^change type of 'p' to 'std::span' to preserve bounds information$
+  int *p = q;  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re^change type of 'p' to 'std::span' to preserve bounds information, and change ('q' and 'r'|'r' and 'q') to 'std::span' to propagate bounds information between them$
   p[5] = 10;  // expected-note{{used in buffer access here}}
-  int *r = q;  // expected-warning{{'r' is an unsafe pointer used for buffer access}} expected-note-re^change type of 'r' to 'std::span' to preserve bounds information$
+  int *r = q;  // expected-warning{{'r' is an unsafe pointer used for buffer access}} expected-note-re^change type of 'r' to 'std::span' to preserve bounds information, and change ('p' and 'q'|'q' and 'p') to 'std::span' to propagate bounds information between them$
   r[10] = 5;  // expected-note{{used in buffer access here}}
 }
 
 void rhs_span2() {
   int *q = new int[6];
-  int *p = q;  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re^change type of 'p' to 'std::span' to preserve bounds information$
+  int *p = q;  // expected-warning{{'p' is an unsafe pointer used for buffer access}}
   p[5] = 10;  // expected-note{{used in buffer access here}}
   int *r = q;
 }
@@ -175,7 +173,7 @@
 void test_crash() {
   int *r = new int[8];
   int *q = r;
-  int *p;  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re^change type of 'p' to 'std::span' to preserve bounds information, and change 'q' to 'std::span' to propagate bounds information between them$
+  int *p;  // expected-warning{{'p' is an unsafe pointer used for buffer access}} expected-note-re^change type of 'p' to 'std::span' to preserve bounds information, and change ('r' and 'q'|'q' and 'r') to 'std::span' to propagate bounds information between them$
   p = q;
   int tmp = p[9];  // expected-note{{used in buffer access here}}
 }
Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp
===
--- clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp
+++ clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp
@@ -38,6 +38,15 @@
   p[5] = 4;  // expected-note{{used in buffer access here}}
 }
 
+void uuc_if_body2_ptr_init(bool flag) {
+  int *r = new int[7];
+  if (flag) {
+  } else {
+int* p = r;  // expected-warning{{'p' is an unsafe pointer used for buffer access}} // expected-note-re^change type of 'p' to 'std::span' to preserve bounds information, and change 'r' to 'std::span' to propagate bounds information between them$
+p[5] 

[clang] db3dced - [-Wunsafe-buffer-usage] Handle pointer initializations for grouping related variables

2023-06-21 Thread Rashmi Mudduluru via cfe-commits

Author: Rashmi Mudduluru
Date: 2023-06-21T15:54:09-07:00
New Revision: db3dcedb9cedcec4a9570fda7406490c642df8ae

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

LOG: [-Wunsafe-buffer-usage] Handle pointer initializations for grouping 
related variables

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

Added: 
clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init-fixits.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-ptr-init.cpp

Modified: 
clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
clang/lib/Analysis/UnsafeBufferUsage.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc-fixits.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-uuc.cpp
clang/test/SemaCXX/warn-unsafe-buffer-usage-multi-decl-warnings.cpp

Removed: 




diff  --git 
a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def 
b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
index 57d9dcc5bdcb7..ff687a0d178bd 100644
--- a/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
+++ b/clang/include/clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def
@@ -37,6 +37,7 @@ FIXABLE_GADGET(UPCAddressofArraySubscript) // '[any]' in 
an Unspecified Poin
 FIXABLE_GADGET(UPCStandalonePointer)
 FIXABLE_GADGET(UPCPreIncrement)// '++Ptr' in an Unspecified 
Pointer Context
 FIXABLE_GADGET(PointerAssignment)
+FIXABLE_GADGET(PointerInit)
 
 #undef FIXABLE_GADGET
 #undef WARNING_GADGET

diff  --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp 
b/clang/lib/Analysis/UnsafeBufferUsage.cpp
index 49229a6b56c1c..aa29fb45726a8 100644
--- a/clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -533,23 +533,66 @@ class PointerArithmeticGadget : public WarningGadget {
   // FIXME: this gadge will need a fix-it
 };
 
+/// A pointer initialization expression of the form:
+///  \code
+///  int *p = q;
+///  \endcode
+class PointerInitGadget : public FixableGadget {
+private:
+  static constexpr const char *const PointerInitLHSTag = "ptrInitLHS";
+  static constexpr const char *const PointerInitRHSTag = "ptrInitRHS";
+  const VarDecl * PtrInitLHS; // the LHS pointer expression in `PI`
+  const DeclRefExpr * PtrInitRHS; // the RHS pointer expression in `PI`
+
+public:
+  PointerInitGadget(const MatchFinder::MatchResult )
+  : FixableGadget(Kind::PointerInit),
+PtrInitLHS(Result.Nodes.getNodeAs(PointerInitLHSTag)),
+PtrInitRHS(Result.Nodes.getNodeAs(PointerInitRHSTag)) {}
+
+  static bool classof(const Gadget *G) {
+return G->getKind() == Kind::PointerInit;
+  }
+
+  static Matcher matcher() {
+auto PtrInitStmt = declStmt(hasSingleDecl(varDecl(
+ hasInitializer(ignoringImpCasts(declRefExpr(
+  hasPointerType()).
+  bind(PointerInitRHSTag.
+  bind(PointerInitLHSTag)));
+
+return stmt(PtrInitStmt);
+  }
+
+  virtual std::optional getFixits(const Strategy ) const override;
+
+  virtual const Stmt *getBaseStmt() const override { return nullptr; }
+
+  virtual DeclUseList getClaimedVarUseSites() const override {
+return DeclUseList{PtrInitRHS};
+  }
+
+  virtual std::optional>
+  getStrategyImplications() const override {
+  return std::make_pair(PtrInitLHS,
+cast(PtrInitRHS->getDecl()));
+  }
+};
+
 /// A pointer assignment expression of the form:
 ///  \code
 ///  p = q;
 ///  \endcode
 class PointerAssignmentGadget : public FixableGadget {
 private:
-  static constexpr const char *const PointerAssignmentTag = "ptrAssign";
   static constexpr const char *const PointerAssignLHSTag = "ptrLHS";
   static constexpr const char *const PointerAssignRHSTag = "ptrRHS";
-  const BinaryOperator *PA;// pointer arithmetic expression
   const DeclRefExpr * PtrLHS; // the LHS pointer expression in `PA`
   const DeclRefExpr * PtrRHS; // the RHS pointer expression in `PA`
 
 public:
   PointerAssignmentGadget(const MatchFinder::MatchResult )
   : FixableGadget(Kind::PointerAssignment),
-PA(Result.Nodes.getNodeAs(PointerAssignmentTag)),
 PtrLHS(Result.Nodes.getNodeAs(PointerAssignLHSTag)),
 PtrRHS(Result.Nodes.getNodeAs(PointerAssignRHSTag)) {}
 
@@ -566,13 +609,12 @@ class PointerAssignmentGadget : public FixableGadget {
   to(varDecl())).
   bind(PointerAssignLHSTag;
 
-//FIXME: Handle declarations at assignments
 return stmt(isInUnspecifiedUntypedContext(PtrAssignExpr));
   }
   
   virtual 

[PATCH] D153176: [Frontend] Remove ShowIncludesPretendHeader

2023-06-21 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1df10f15807f: [Frontend] Remove ShowIncludesPretendHeader 
(authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153176

Files:
  clang/include/clang/Frontend/DependencyOutputOptions.h
  clang/lib/Frontend/HeaderIncludeGen.cpp


Index: clang/lib/Frontend/HeaderIncludeGen.cpp
===
--- clang/lib/Frontend/HeaderIncludeGen.cpp
+++ clang/lib/Frontend/HeaderIncludeGen.cpp
@@ -214,13 +214,8 @@
 
 // We track when we are done with the predefines by watching for the first
 // place where we drop back to a nesting depth of 1.
-if (CurrentIncludeDepth == 1 && !HasProcessedPredefines) {
-  if (!DepOpts.ShowIncludesPretendHeader.empty()) {
-PrintHeaderInfo(OutputFile, DepOpts.ShowIncludesPretendHeader,
-ShowDepth, 2, MSStyle);
-  }
+if (CurrentIncludeDepth == 1 && !HasProcessedPredefines)
   HasProcessedPredefines = true;
-}
 
 return;
   } else {
@@ -233,8 +228,6 @@
   unsigned IncludeDepth = CurrentIncludeDepth;
   if (!HasProcessedPredefines)
 --IncludeDepth; // Ignore indent from .
-  else if (!DepOpts.ShowIncludesPretendHeader.empty())
-++IncludeDepth; // Pretend inclusion by ShowIncludesPretendHeader.
 
   // FIXME: Identify headers in a more robust way than comparing their name to
   // "" and "" in a bunch of places.
Index: clang/include/clang/Frontend/DependencyOutputOptions.h
===
--- clang/include/clang/Frontend/DependencyOutputOptions.h
+++ clang/include/clang/Frontend/DependencyOutputOptions.h
@@ -76,9 +76,6 @@
   /// target.
   std::vector> ExtraDeps;
 
-  /// In /showIncludes mode, pretend the main TU is a header with this name.
-  std::string ShowIncludesPretendHeader;
-
   /// The file to write GraphViz-formatted header dependencies to.
   std::string DOTOutputFile;
 


Index: clang/lib/Frontend/HeaderIncludeGen.cpp
===
--- clang/lib/Frontend/HeaderIncludeGen.cpp
+++ clang/lib/Frontend/HeaderIncludeGen.cpp
@@ -214,13 +214,8 @@
 
 // We track when we are done with the predefines by watching for the first
 // place where we drop back to a nesting depth of 1.
-if (CurrentIncludeDepth == 1 && !HasProcessedPredefines) {
-  if (!DepOpts.ShowIncludesPretendHeader.empty()) {
-PrintHeaderInfo(OutputFile, DepOpts.ShowIncludesPretendHeader,
-ShowDepth, 2, MSStyle);
-  }
+if (CurrentIncludeDepth == 1 && !HasProcessedPredefines)
   HasProcessedPredefines = true;
-}
 
 return;
   } else {
@@ -233,8 +228,6 @@
   unsigned IncludeDepth = CurrentIncludeDepth;
   if (!HasProcessedPredefines)
 --IncludeDepth; // Ignore indent from .
-  else if (!DepOpts.ShowIncludesPretendHeader.empty())
-++IncludeDepth; // Pretend inclusion by ShowIncludesPretendHeader.
 
   // FIXME: Identify headers in a more robust way than comparing their name to
   // "" and "" in a bunch of places.
Index: clang/include/clang/Frontend/DependencyOutputOptions.h
===
--- clang/include/clang/Frontend/DependencyOutputOptions.h
+++ clang/include/clang/Frontend/DependencyOutputOptions.h
@@ -76,9 +76,6 @@
   /// target.
   std::vector> ExtraDeps;
 
-  /// In /showIncludes mode, pretend the main TU is a header with this name.
-  std::string ShowIncludesPretendHeader;
-
   /// The file to write GraphViz-formatted header dependencies to.
   std::string DOTOutputFile;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D153175: [Frontend] Don't output skipped includes from predefines

2023-06-21 Thread Shoaib Meenai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG67a11290df64: [Frontend] Dont output skipped includes 
from predefines (authored by smeenai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153175

Files:
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/test/Frontend/print-header-includes.c

Index: clang/test/Frontend/print-header-includes.c
===
--- clang/test/Frontend/print-header-includes.c
+++ clang/test/Frontend/print-header-includes.c
@@ -17,6 +17,29 @@
 // SKIPPED: .. {{.*test2.h}}
 // SKIPPED: .. {{.*test2.h}}
 
+// RUN: %clang_cc1 -isystem %S -isystem %S/Inputs/SystemHeaderPrefix \
+// RUN: -E -H -fshow-skipped-includes -sys-header-deps -o /dev/null %s 2> %t.stderr
+// RUN: FileCheck --check-prefix=SKIPPED-SYS < %t.stderr %s
+
+// SKIPPED-SYS: . {{.*noline.h}}
+// SKIPPED-SYS: . {{.*test.h}}
+// SKIPPED-SYS: .. {{.*test2.h}}
+// SKIPPED-SYS: .. {{.*test2.h}}
+
+// RUN: %clang_cc1 -I%S -isystem %S/Inputs/SystemHeaderPrefix -include Inputs/test.h \
+// RUN: -E -H -fshow-skipped-includes -o /dev/null %s 2> %t.stderr
+// RUN: FileCheck --check-prefix=SKIPPED-PREDEFINES < %t.stderr %s
+
+// The skipped include of test2.h from the -include test.h shouldn't be printed.
+// SKIPPED-PREDEFINES-NOT: {{.*test2.h}}
+// SKIPPED-PREDEFINES: . {{.*test.h}}
+
+// RUN: %clang_cc1 -isystem %S -isystem %S/Inputs/SystemHeaderPrefix \
+// RUN: -E -H -fshow-skipped-includes -o /dev/null %s 2> %t.stderr
+// RUN: FileCheck --check-prefix=SKIPPED-NO-SYS --allow-empty < %t.stderr %s
+
+// SKIPPED-NO-SYS-NOT: .
+
 // RUN: %clang_cc1 -I%S -include Inputs/test3.h -isystem %S/Inputs/SystemHeaderPrefix \
 // RUN: -E -H -sys-header-deps -o /dev/null %s 2> %t.stderr
 // RUN: FileCheck --check-prefix SYSHEADERS < %t.stderr %s
@@ -58,4 +81,4 @@
 // MS-IGNORELIST-NOT: Note
 
 #include 
-#include "Inputs/test.h"
+#include 
Index: clang/lib/Frontend/HeaderIncludeGen.cpp
===
--- clang/lib/Frontend/HeaderIncludeGen.cpp
+++ clang/lib/Frontend/HeaderIncludeGen.cpp
@@ -49,6 +49,18 @@
 
   void FileSkipped(const FileEntryRef , const Token ,
SrcMgr::CharacteristicKind FileType) override;
+
+private:
+  bool ShouldShowHeader(SrcMgr::CharacteristicKind HeaderType) {
+if (!DepOpts.IncludeSystemHeaders && isSystem(HeaderType))
+  return false;
+
+// Show the current header if we are (a) past the predefines, or (b) showing
+// all headers and in the predefines at a depth past the initial file and
+// command line buffers.
+return (HasProcessedPredefines ||
+(ShowAllHeaders && CurrentIncludeDepth > 2));
+  }
 };
 
 /// A callback for emitting header usage information to a file in JSON. Each
@@ -211,29 +223,22 @@
 }
 
 return;
-  } else
+  } else {
+return;
+  }
+
+  if (!ShouldShowHeader(NewFileType))
 return;
 
-  // Show the header if we are (a) past the predefines, or (b) showing all
-  // headers and in the predefines at a depth past the initial file and command
-  // line buffers.
-  bool ShowHeader = (HasProcessedPredefines ||
- (ShowAllHeaders && CurrentIncludeDepth > 2));
   unsigned IncludeDepth = CurrentIncludeDepth;
   if (!HasProcessedPredefines)
 --IncludeDepth; // Ignore indent from .
   else if (!DepOpts.ShowIncludesPretendHeader.empty())
 ++IncludeDepth; // Pretend inclusion by ShowIncludesPretendHeader.
 
-  if (!DepOpts.IncludeSystemHeaders && isSystem(NewFileType))
-ShowHeader = false;
-
-  // Dump the header include information we are past the predefines buffer or
-  // are showing all headers and this isn't the magic implicit 
-  // header.
   // FIXME: Identify headers in a more robust way than comparing their name to
   // "" and "" in a bunch of places.
-  if (ShowHeader && Reason == PPCallbacks::EnterFile &&
+  if (Reason == PPCallbacks::EnterFile &&
   UserLoc.getFilename() != StringRef("")) {
 PrintHeaderInfo(OutputFile, UserLoc.getFilename(), ShowDepth, IncludeDepth,
 MSStyle);
@@ -246,7 +251,7 @@
   if (!DepOpts.ShowSkippedHeaderIncludes)
 return;
 
-  if (!DepOpts.IncludeSystemHeaders && isSystem(FileType))
+  if (!ShouldShowHeader(FileType))
 return;
 
   PrintHeaderInfo(OutputFile, SkippedFile.getName(), ShowDepth,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 1df10f1 - [Frontend] Remove ShowIncludesPretendHeader

2023-06-21 Thread Shoaib Meenai via cfe-commits

Author: Shoaib Meenai
Date: 2023-06-21T15:48:28-07:00
New Revision: 1df10f15807f9b7deba3f714d27e21578a8b4748

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

LOG: [Frontend] Remove ShowIncludesPretendHeader

It hasn't been written to since https://reviews.llvm.org/D46652, so it
was always empty. I don't have enough context on that diff to know if
the removal of the write to ShowIncludesPretendHeader in that diff was
intentional, but no one's complained about it for five years, so I
assume we're okay to just get rid of it entirely.

Reviewed By: hans

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

Added: 


Modified: 
clang/include/clang/Frontend/DependencyOutputOptions.h
clang/lib/Frontend/HeaderIncludeGen.cpp

Removed: 




diff  --git a/clang/include/clang/Frontend/DependencyOutputOptions.h 
b/clang/include/clang/Frontend/DependencyOutputOptions.h
index e140ff9baa117..e4b26d92647d0 100644
--- a/clang/include/clang/Frontend/DependencyOutputOptions.h
+++ b/clang/include/clang/Frontend/DependencyOutputOptions.h
@@ -76,9 +76,6 @@ class DependencyOutputOptions {
   /// target.
   std::vector> ExtraDeps;
 
-  /// In /showIncludes mode, pretend the main TU is a header with this name.
-  std::string ShowIncludesPretendHeader;
-
   /// The file to write GraphViz-formatted header dependencies to.
   std::string DOTOutputFile;
 

diff  --git a/clang/lib/Frontend/HeaderIncludeGen.cpp 
b/clang/lib/Frontend/HeaderIncludeGen.cpp
index 27cd8b701a975..9ada227d42d62 100644
--- a/clang/lib/Frontend/HeaderIncludeGen.cpp
+++ b/clang/lib/Frontend/HeaderIncludeGen.cpp
@@ -214,13 +214,8 @@ void HeaderIncludesCallback::FileChanged(SourceLocation 
Loc,
 
 // We track when we are done with the predefines by watching for the first
 // place where we drop back to a nesting depth of 1.
-if (CurrentIncludeDepth == 1 && !HasProcessedPredefines) {
-  if (!DepOpts.ShowIncludesPretendHeader.empty()) {
-PrintHeaderInfo(OutputFile, DepOpts.ShowIncludesPretendHeader,
-ShowDepth, 2, MSStyle);
-  }
+if (CurrentIncludeDepth == 1 && !HasProcessedPredefines)
   HasProcessedPredefines = true;
-}
 
 return;
   } else {
@@ -233,8 +228,6 @@ void HeaderIncludesCallback::FileChanged(SourceLocation Loc,
   unsigned IncludeDepth = CurrentIncludeDepth;
   if (!HasProcessedPredefines)
 --IncludeDepth; // Ignore indent from .
-  else if (!DepOpts.ShowIncludesPretendHeader.empty())
-++IncludeDepth; // Pretend inclusion by ShowIncludesPretendHeader.
 
   // FIXME: Identify headers in a more robust way than comparing their name to
   // "" and "" in a bunch of places.



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


[clang] 67a1129 - [Frontend] Don't output skipped includes from predefines

2023-06-21 Thread Shoaib Meenai via cfe-commits

Author: Shoaib Meenai
Date: 2023-06-21T15:48:27-07:00
New Revision: 67a11290df64fec44e671a1bdc3a225ed8a02962

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

LOG: [Frontend] Don't output skipped includes from predefines

`-H` displays a tree of included header files, but that tree is supposed
to omit two categories of header files:
1. Any header files pulled in via `-include`, which the code refers to
   as the "predefines".
2. Any header files whose inclusion was skipped because they'd already
   been included (assuming header guards or `#pragma once`).

`-fshow-skipped-includes` was intended to make `-H` display the second
category of files. It wasn't checking for the first category, however,
so you could end up with only the middle of the `-include` hierarchy
displayed, e.g. the added test would previously output:

```
... /data/users/smeenai/llvm-project/clang/test/Frontend/Inputs/test2.h
. /data/users/smeenai/llvm-project/clang/test/Frontend/Inputs/test.h
```

This diff adds a check to prevent that and correctly omit headers from
`-include` even when `-fshow-skipped-includes` is passed. While I'm
here, add tests for the interaction between `-fshow-skipped-includes`
and `-sys-header-deps` as well.

Reviewed By: hans

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

Added: 


Modified: 
clang/lib/Frontend/HeaderIncludeGen.cpp
clang/test/Frontend/print-header-includes.c

Removed: 




diff  --git a/clang/lib/Frontend/HeaderIncludeGen.cpp 
b/clang/lib/Frontend/HeaderIncludeGen.cpp
index 2ab4809402645..27cd8b701a975 100644
--- a/clang/lib/Frontend/HeaderIncludeGen.cpp
+++ b/clang/lib/Frontend/HeaderIncludeGen.cpp
@@ -49,6 +49,18 @@ class HeaderIncludesCallback : public PPCallbacks {
 
   void FileSkipped(const FileEntryRef , const Token ,
SrcMgr::CharacteristicKind FileType) override;
+
+private:
+  bool ShouldShowHeader(SrcMgr::CharacteristicKind HeaderType) {
+if (!DepOpts.IncludeSystemHeaders && isSystem(HeaderType))
+  return false;
+
+// Show the current header if we are (a) past the predefines, or (b) 
showing
+// all headers and in the predefines at a depth past the initial file and
+// command line buffers.
+return (HasProcessedPredefines ||
+(ShowAllHeaders && CurrentIncludeDepth > 2));
+  }
 };
 
 /// A callback for emitting header usage information to a file in JSON. Each
@@ -211,29 +223,22 @@ void HeaderIncludesCallback::FileChanged(SourceLocation 
Loc,
 }
 
 return;
-  } else
+  } else {
+return;
+  }
+
+  if (!ShouldShowHeader(NewFileType))
 return;
 
-  // Show the header if we are (a) past the predefines, or (b) showing all
-  // headers and in the predefines at a depth past the initial file and command
-  // line buffers.
-  bool ShowHeader = (HasProcessedPredefines ||
- (ShowAllHeaders && CurrentIncludeDepth > 2));
   unsigned IncludeDepth = CurrentIncludeDepth;
   if (!HasProcessedPredefines)
 --IncludeDepth; // Ignore indent from .
   else if (!DepOpts.ShowIncludesPretendHeader.empty())
 ++IncludeDepth; // Pretend inclusion by ShowIncludesPretendHeader.
 
-  if (!DepOpts.IncludeSystemHeaders && isSystem(NewFileType))
-ShowHeader = false;
-
-  // Dump the header include information we are past the predefines buffer or
-  // are showing all headers and this isn't the magic implicit 
-  // header.
   // FIXME: Identify headers in a more robust way than comparing their name to
   // "" and "" in a bunch of places.
-  if (ShowHeader && Reason == PPCallbacks::EnterFile &&
+  if (Reason == PPCallbacks::EnterFile &&
   UserLoc.getFilename() != StringRef("")) {
 PrintHeaderInfo(OutputFile, UserLoc.getFilename(), ShowDepth, IncludeDepth,
 MSStyle);
@@ -246,7 +251,7 @@ void HeaderIncludesCallback::FileSkipped(const FileEntryRef 
, const
   if (!DepOpts.ShowSkippedHeaderIncludes)
 return;
 
-  if (!DepOpts.IncludeSystemHeaders && isSystem(FileType))
+  if (!ShouldShowHeader(FileType))
 return;
 
   PrintHeaderInfo(OutputFile, SkippedFile.getName(), ShowDepth,

diff  --git a/clang/test/Frontend/print-header-includes.c 
b/clang/test/Frontend/print-header-includes.c
index c210e53ee96c6..77db5ad108e39 100644
--- a/clang/test/Frontend/print-header-includes.c
+++ b/clang/test/Frontend/print-header-includes.c
@@ -17,6 +17,29 @@
 // SKIPPED: .. {{.*test2.h}}
 // SKIPPED: .. {{.*test2.h}}
 
+// RUN: %clang_cc1 -isystem %S -isystem %S/Inputs/SystemHeaderPrefix \
+// RUN: -E -H -fshow-skipped-includes -sys-header-deps -o /dev/null %s 2> 
%t.stderr
+// RUN: FileCheck --check-prefix=SKIPPED-SYS < %t.stderr %s
+
+// SKIPPED-SYS: . {{.*noline.h}}
+// SKIPPED-SYS: . {{.*test.h}}
+// SKIPPED-SYS: .. {{.*test2.h}}
+// SKIPPED-SYS: 

[PATCH] D150803: Add a new `wasm_custom` clang attribute for marking functions.

2023-06-21 Thread Brendan Dahl via Phabricator via cfe-commits
brendandahl added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:5608-5612
+Clang supports the ``__attribute__((wasm_async))``
+attribute for the WebAssembly target. This attribute may be attached to a
+function definition, which indicates the function will be used with JavaScript
+promise integration (JSPI). The attribute will cause the creation of a custom
+section named "async" that contains each wasm_async function's index value.

aaron.ballman wrote:
> aaron.ballman wrote:
> > This could be my ignorance of web assembly showing, but the docs don't 
> > really help me understand when you'd want to use this attribute. Perhaps a 
> > link to what JSPI is and a code example would help a little bit? Or is this 
> > more of a low-level implementation detail kind of attribute where folks 
> > already know the domain?
> Based on the documentation here, I'm wondering why the `annotate` attribute 
> doesn't suffice? That attribute lets you specify custom information to 
> associate with a declaration that then gets lowered such that passes can do 
> whatever they want with the info, which seems to be a more generalized 
> version of what this attribute is.
> 
> (FWIW, I'm back to having basically no idea when you'd use this attribute or 
> what it would be used for, so my thoughts above might make no sense.)
I was considering that, but it would require more machinery in the wasm backend 
to store all the attribute values in the output. For now we only really need a 
flag associated with function. I think if we find more uses for the annotations 
in the future we could replace wasm_custom with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150803

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


[PATCH] D153476: [dataflow] document & test determinism of formula structure

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
Herald added subscribers: martong, mgrang, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This doesn't depend on object addresses, even though they are used
in canonicalization. This is important & subtle, so add a comment+test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153476

Files:
  clang/lib/Analysis/FlowSensitive/Arena.cpp
  clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
@@ -147,5 +147,28 @@
   EXPECT_EQ(, );
 }
 
+// Tests that the structure of the formula we end up with does not depend on
+// details like the pointer address of internal values.
+TEST_F(ArenaTest, Canonicalization) {
+  StringRef Expected = "((V0 | V1) = (V1 & V0))";
+
+  auto  = A.create();
+  auto  = A.create();
+  auto  = A.makeOr(AX, AY);
+  auto  = A.makeAnd(AY, AX);
+  auto  = A.makeEquals(AOr, AAnd);
+  EXPECT_EQ(Expected, llvm::to_string(A.getFormula(AEqual)));
+
+  // Same, but create the & clause first.
+  // The relative order of (, ) and (, ) likely differs.
+  Arena B;
+  auto  = A.create();
+  auto  = A.create();
+  auto  = A.makeAnd(BX, BY);
+  auto  = A.makeOr(BY, BX);
+  auto  = A.makeEquals(BOr, BAnd);
+  EXPECT_EQ(Expected, llvm::to_string(B.getFormula(BEqual)));
+}
+
 } // namespace
 } // namespace clang::dataflow
Index: clang/lib/Analysis/FlowSensitive/Arena.cpp
===
--- clang/lib/Analysis/FlowSensitive/Arena.cpp
+++ clang/lib/Analysis/FlowSensitive/Arena.cpp
@@ -10,6 +10,10 @@
 
 namespace clang::dataflow {
 
+// Compute a canonical key for an unordered poir of operands, so that we
+// can intern (A & B) and (B & A) to the same formula.
+// This does not affect the actual order used in the formula, which is always
+// that of the first call to makeAnd().
 static std::pair
 makeCanonicalBoolValuePair(BoolValue , BoolValue ) {
   auto Res = std::make_pair(, );


Index: clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
@@ -147,5 +147,28 @@
   EXPECT_EQ(, );
 }
 
+// Tests that the structure of the formula we end up with does not depend on
+// details like the pointer address of internal values.
+TEST_F(ArenaTest, Canonicalization) {
+  StringRef Expected = "((V0 | V1) = (V1 & V0))";
+
+  auto  = A.create();
+  auto  = A.create();
+  auto  = A.makeOr(AX, AY);
+  auto  = A.makeAnd(AY, AX);
+  auto  = A.makeEquals(AOr, AAnd);
+  EXPECT_EQ(Expected, llvm::to_string(A.getFormula(AEqual)));
+
+  // Same, but create the & clause first.
+  // The relative order of (, ) and (, ) likely differs.
+  Arena B;
+  auto  = A.create();
+  auto  = A.create();
+  auto  = A.makeAnd(BX, BY);
+  auto  = A.makeOr(BY, BX);
+  auto  = A.makeEquals(BOr, BAnd);
+  EXPECT_EQ(Expected, llvm::to_string(B.getFormula(BEqual)));
+}
+
 } // namespace
 } // namespace clang::dataflow
Index: clang/lib/Analysis/FlowSensitive/Arena.cpp
===
--- clang/lib/Analysis/FlowSensitive/Arena.cpp
+++ clang/lib/Analysis/FlowSensitive/Arena.cpp
@@ -10,6 +10,10 @@
 
 namespace clang::dataflow {
 
+// Compute a canonical key for an unordered poir of operands, so that we
+// can intern (A & B) and (B & A) to the same formula.
+// This does not affect the actual order used in the formula, which is always
+// that of the first call to makeAnd().
 static std::pair
 makeCanonicalBoolValuePair(BoolValue , BoolValue ) {
   auto Res = std::make_pair(, );
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D153175: [Frontend] Don't output skipped includes from predefines

2023-06-21 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment.

In D153175#4431923 , @hans wrote:

>> -H is supposed to skip outputting headers from -include command line
>> arguments, but -fshow-skipped-includes was outputting any skipped
>> includes encountered via -include.
>
> I was thrown off by the "-H is supposed to skip ... but 
> -fshow-skipped-includes was outputting ..."
>
> I suppose the point is that we do want `-fshow-skipped-includes` to show 
> skipped includes, but not the ones from `-include` or system headers (unless 
> `-sys-header-deps`)?
>
> lgtm if I got that right.

Yup, my description was kinda terrible :D Lemme try to clarify it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153175

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


[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

2023-06-21 Thread Owen Pan via Phabricator via cfe-commits
owenpan marked 2 inline comments as done.
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:6371-6380
+  verifyFormat("#if FOO\n"
+   "int a = 1;\n"
+   "#else\n"
+   "int ab = 2;\n"
+   "#endif\n"
+   "#ifdef BAR\n"
+   "int abc = 3;\n"

owenpan wrote:
> HazardyKnusperkeks wrote:
> > owenpan wrote:
> > > clang-format breaks the above into two separate sets:
> > > ```
> > > #if FOO
> > > int a = 1;
> > > #else
> > > #endif
> > > #if BAR
> > > int abc = 3;
> > > #else
> > > #endif
> > > ```
> > > and:
> > > ```
> > > #if FOO
> > > #else
> > > int ab = 2;
> > > #endif
> > > #if BAR
> > > #else
> > > int abcd = 4;
> > > #endif
> > > ```
> > > After it finishes with the first set, the preprocessor directives are 
> > > marked as `Finalized`. Then, while the second set is being processed, the 
> > > directives should //not// be skipped when tokens are added to the 
> > > `Change` set. Otherwise, we would end up with the `Change` set below 
> > > without any "scope" context:
> > > ```
> > > int ab = 2;
> > > int abcd = 4;
> > > ```
> > Fascinating. But wouldn't the better fix be that the directives are not 
> > marked as finalized?
> I tried that first, but it broke a lot of unit tests. I'll give it another 
> try when I have time. :)
See D153243.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150057

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


[PATCH] D153243: [clang-format] Don't finalize #if, #else, #endif, etc.

2023-06-21 Thread Owen Pan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8e85739a5f24: [clang-format] Dont finalize #if, #else, 
#endif, etc. (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153243

Files:
  clang/lib/Format/UnwrappedLineFormatter.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
@@ -23106,6 +23106,13 @@
"int* j;\n"
"// clang-format only\n"
"int* k;");
+
+  verifyNoChange("// clang-format off\n"
+ "#if 0\n"
+ "#if SHOULD_STAY_INDENTED\n"
+ " #endif\n"
+ "#endif\n"
+ "// clang-format on");
 }
 
 TEST_F(FormatTest, DoNotCrashOnInvalidInput) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -49,16 +49,8 @@
   unsigned Spaces,
   unsigned StartOfTokenColumn,
   bool IsAligned, bool InPPDirective) {
-  auto PPBranchDirectiveStartsLine = [] {
-return Tok.is(tok::hash) && !Tok.Previous && Tok.Next &&
-   Tok.Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
- tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
- tok::pp_else, tok::pp_endif);
-  };
-  if ((Tok.Finalized && !PPBranchDirectiveStartsLine()) ||
-  (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg)) {
+  if (Tok.Finalized || (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg))
 return;
-  }
   Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
   Changes.push_back(Change(Tok, /*CreateReplacement=*/true, 
Tok.WhitespaceRange,
Spaces, StartOfTokenColumn, Newlines, "", "",
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1411,8 +1411,16 @@
   NextLine = Joiner.getNextMergedLine(DryRun, IndentTracker);
   RangeMinLevel = UINT_MAX;
 }
-if (!DryRun)
-  markFinalized(TheLine.First);
+if (!DryRun) {
+  auto *Tok = TheLine.First;
+  if (Tok->is(tok::hash) && !Tok->Previous && Tok->Next &&
+  Tok->Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
+ tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
+ tok::pp_else, tok::pp_endif)) {
+Tok = Tok->Next;
+  }
+  markFinalized(Tok);
+}
   }
   PenaltyCache[CacheKey] = Penalty;
   return Penalty;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -23106,6 +23106,13 @@
"int* j;\n"
"// clang-format only\n"
"int* k;");
+
+  verifyNoChange("// clang-format off\n"
+ "#if 0\n"
+ "#if SHOULD_STAY_INDENTED\n"
+ " #endif\n"
+ "#endif\n"
+ "// clang-format on");
 }
 
 TEST_F(FormatTest, DoNotCrashOnInvalidInput) {
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -49,16 +49,8 @@
   unsigned Spaces,
   unsigned StartOfTokenColumn,
   bool IsAligned, bool InPPDirective) {
-  auto PPBranchDirectiveStartsLine = [] {
-return Tok.is(tok::hash) && !Tok.Previous && Tok.Next &&
-   Tok.Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
- tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
- tok::pp_else, tok::pp_endif);
-  };
-  if ((Tok.Finalized && !PPBranchDirectiveStartsLine()) ||
-  (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg)) {
+  if (Tok.Finalized || (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg))
 return;
-  }
   Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
   Changes.push_back(Change(Tok, /*CreateReplacement=*/true, Tok.WhitespaceRange,
Spaces, StartOfTokenColumn, Newlines, "", "",
Index: clang/lib/Format/UnwrappedLineFormatter.cpp

[clang] 8e85739 - [clang-format] Don't finalize #if, #else, #endif, etc.

2023-06-21 Thread Owen Pan via cfe-commits

Author: Owen Pan
Date: 2023-06-21T15:15:57-07:00
New Revision: 8e85739a5f24e34cb22768fe6ef33b593ed054df

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

LOG: [clang-format] Don't finalize #if, #else, #endif, etc.

Don't finalize a preprocessor branch directive if it's the first
token of an annotated line. See the rationale at
https://reviews.llvm.org/D150057#inline-1449546.

Fixes #63379

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/WhitespaceManager.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineFormatter.cpp 
b/clang/lib/Format/UnwrappedLineFormatter.cpp
index 6d9a43dea0568..3eacd0201df32 100644
--- a/clang/lib/Format/UnwrappedLineFormatter.cpp
+++ b/clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -1411,8 +1411,16 @@ unsigned UnwrappedLineFormatter::format(
   NextLine = Joiner.getNextMergedLine(DryRun, IndentTracker);
   RangeMinLevel = UINT_MAX;
 }
-if (!DryRun)
-  markFinalized(TheLine.First);
+if (!DryRun) {
+  auto *Tok = TheLine.First;
+  if (Tok->is(tok::hash) && !Tok->Previous && Tok->Next &&
+  Tok->Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
+ tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
+ tok::pp_else, tok::pp_endif)) {
+Tok = Tok->Next;
+  }
+  markFinalized(Tok);
+}
   }
   PenaltyCache[CacheKey] = Penalty;
   return Penalty;

diff  --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 040b9536b6305..24255522263ad 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -49,16 +49,8 @@ void WhitespaceManager::replaceWhitespace(FormatToken , 
unsigned Newlines,
   unsigned Spaces,
   unsigned StartOfTokenColumn,
   bool IsAligned, bool InPPDirective) {
-  auto PPBranchDirectiveStartsLine = [] {
-return Tok.is(tok::hash) && !Tok.Previous && Tok.Next &&
-   Tok.Next->isOneOf(tok::pp_if, tok::pp_ifdef, tok::pp_ifndef,
- tok::pp_elif, tok::pp_elifdef, tok::pp_elifndef,
- tok::pp_else, tok::pp_endif);
-  };
-  if ((Tok.Finalized && !PPBranchDirectiveStartsLine()) ||
-  (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg)) {
+  if (Tok.Finalized || (Tok.MacroCtx && Tok.MacroCtx->Role == MR_ExpandedArg))
 return;
-  }
   Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
   Changes.push_back(Change(Tok, /*CreateReplacement=*/true, 
Tok.WhitespaceRange,
Spaces, StartOfTokenColumn, Newlines, "", "",

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 4c58df893aa8f..6b0540400d7b7 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -23106,6 +23106,13 @@ TEST_F(FormatTest, DisableRegions) {
"int* j;\n"
"// clang-format only\n"
"int* k;");
+
+  verifyNoChange("// clang-format off\n"
+ "#if 0\n"
+ "#if SHOULD_STAY_INDENTED\n"
+ " #endif\n"
+ "#endif\n"
+ "// clang-format on");
 }
 
 TEST_F(FormatTest, DoNotCrashOnInvalidInput) {



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


[PATCH] D153469: [dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

2023-06-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This properly frees the Value hierarchy from managing boolean formulas.

We still distinguish AtomicBoolValue; this type is used in client code.
However we expect to convert such uses to BoolValue (where the
distinction is not needed) or Atom (where atomic identity is intended),
and then fold AtomicBoolValue into FormulaBoolValue.

We also distinguish TopBoolValue; this has distinct rules for
widen/join/equivalence, and top-ness is not represented in Formula.
It'd be nice to find a cleaner representation (e.g. the absence of a
formula), but no immediate plans.

For now, BoolValues with the same Formula are deduplicated. This doesn't
seem desirable, as Values are mutable by their creators (properties).
We can probably drop this for FormulaBoolValue immediately (not in this
patch, to isolate changes). For AtomicBoolValue we first need to update
clients to stop using value pointers for atom identity.

The data structures around flow conditions are updated:

- flow condition tokens are Atom, rather than AtomicBoolValue*
- conditions are Formula, rather than BoolValue

Most APIs were changed directly, some with many clients had a
new version added and the existing one deprecated.

The factories for BoolValues in Environment keep their existing
signatures for now (e.g. makeOr(BoolValue, BoolValue) => BoolValue)
and are not deprecated. These have very many clients and finding the
most ergonomic API & migration path still needs some thought.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153469

Files:
  clang/include/clang/Analysis/FlowSensitive/Arena.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/Arena.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowAnalysisContextTest.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/ValueTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/ValueTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Analysis/FlowSensitive/Arena.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -39,17 +40,19 @@
 }
 
 TEST(ValueTest, TopsEquivalent) {
-  TopBoolValue V1;
-  TopBoolValue V2;
+  Arena A;
+  TopBoolValue V1(A.makeAtomRef(Atom(0)));
+  TopBoolValue V2(A.makeAtomRef(Atom(1)));
   EXPECT_TRUE(areEquivalentValues(V1, V2));
   EXPECT_TRUE(areEquivalentValues(V2, V1));
 }
 
 TEST(ValueTest, EquivalentValuesWithDifferentPropsEquivalent) {
-  TopBoolValue Prop1;
-  TopBoolValue Prop2;
-  TopBoolValue V1;
-  TopBoolValue V2;
+  Arena A;
+  TopBoolValue Prop1(A.makeAtomRef(Atom(0)));
+  TopBoolValue Prop2(A.makeAtomRef(Atom(1)));
+  TopBoolValue V1(A.makeAtomRef(Atom(2)));
+  TopBoolValue V2(A.makeAtomRef(Atom(3)));
   V1.setProperty("foo", Prop1);
   V2.setProperty("bar", Prop2);
   EXPECT_TRUE(areEquivalentValues(V1, V2));
@@ -57,9 +60,10 @@
 }
 
 TEST(ValueTest, DifferentKindsNotEquivalent) {
+  Arena A;
   auto L = ScalarStorageLocation(QualType());
   ReferenceValue V1(L);
-  TopBoolValue V2;
+  TopBoolValue V2(A.makeAtomRef(Atom(0)));
   EXPECT_FALSE(areEquivalentValues(V1, V2));
   EXPECT_FALSE(areEquivalentValues(V2, V1));
 }
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2908,14 +2908,12 @@
   ASSERT_THAT(BazDecl, NotNull());
 
   const auto *BazVal =
-  dyn_cast_or_null(Env.getValue(*BazDecl));
+  dyn_cast_or_null(Env.getValue(*BazDecl));
   ASSERT_THAT(BazVal, NotNull());
-  EXPECT_EQ(>getLeftSubValue(), FooVal);
-
-  const auto *BazRightSubValVal =
-  cast(>getRightSubValue());
-  EXPECT_EQ(>getLeftSubValue(), BarVal);
-  EXPECT_EQ(>getRightSubValue(), QuxVal);
+  auto  = Env.arena();
+   

[PATCH] D153468: [clang][LTO] Add flag to run verifier after every pass

2023-06-21 Thread Arthur Eubanks via Phabricator via cfe-commits
aeubanks created this revision.
aeubanks added a reviewer: tejohnson.
Herald added subscribers: ormris, steven_wu, hiraditya, inglorion.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Helps with debugging issues caught by the verifier.

Plumbed through both normal clang compile and ThinLTO.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153468

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/verify-each.c
  llvm/include/llvm/LTO/Config.h
  llvm/lib/LTO/LTOBackend.cpp


Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -259,7 +259,8 @@
   ModuleAnalysisManager MAM;
 
   PassInstrumentationCallbacks PIC;
-  StandardInstrumentations SI(Mod.getContext(), Conf.DebugPassManager);
+  StandardInstrumentations SI(Mod.getContext(), Conf.DebugPassManager,
+  Conf.VerifyEach);
   SI.registerCallbacks(PIC, );
   PassBuilder PB(TM, Conf.PTO, PGOOpt, );
 
Index: llvm/include/llvm/LTO/Config.h
===
--- llvm/include/llvm/LTO/Config.h
+++ llvm/include/llvm/LTO/Config.h
@@ -57,6 +57,7 @@
   CodeGenOpt::Level CGOptLevel = CodeGenOpt::Default;
   CodeGenFileType CGFileType = CGFT_ObjectFile;
   unsigned OptLevel = 2;
+  bool VerifyEach = false;
   bool DisableVerify = false;
 
   /// Use the standard optimization pipeline.
Index: clang/test/CodeGen/verify-each.c
===
--- /dev/null
+++ clang/test/CodeGen/verify-each.c
@@ -0,0 +1,17 @@
+// Test to ensure -llvm-verify-each works.
+// REQUIRES: x86-registered-target
+
+// RUN: %clang_cc1 -O2 -o /dev/null -triple x86_64-unknown-linux-gnu 
-emit-llvm-bc %s -fdebug-pass-manager 2>&1 | FileCheck %s --check-prefix=NO
+// NO-NOT: Verifying
+
+// RUN: %clang_cc1 -O2 -o /dev/null -triple x86_64-unknown-linux-gnu 
-emit-llvm-bc %s -fdebug-pass-manager -llvm-verify-each 2>&1 | FileCheck %s
+
+// RUN: %clang_cc1 -O2 -o %t.o -flto=thin -triple x86_64-unknown-linux-gnu 
-emit-llvm-bc %s -fdebug-pass-manager -llvm-verify-each 2>&1 | FileCheck %s
+// RUN: llvm-lto -thinlto -o %t %t.o
+
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-obj -O2 -o /dev/null 
-x ir %t.o -fthinlto-index=%t.thinlto.bc -fdebug-pass-manager -llvm-verify-each 
2>&1 | FileCheck %s
+
+// CHECK: Verifying
+
+void foo(void) {
+}
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -839,7 +839,7 @@
   StandardInstrumentations SI(
   TheModule->getContext(),
   (CodeGenOpts.DebugPassManager || DebugPassStructure),
-  /*VerifyEach*/ false, PrintPassOpts);
+  CodeGenOpts.VerifyEach, PrintPassOpts);
   SI.registerCallbacks(PIC, );
   PassBuilder PB(TM.get(), PTO, PGOOpt, );
 
@@ -1192,6 +1192,7 @@
 
   Conf.ProfileRemapping = std::move(ProfileRemapping);
   Conf.DebugPassManager = CGOpts.DebugPassManager;
+  Conf.VerifyEach = CGOpts.VerifyEach;
   Conf.RemarksWithHotness = CGOpts.DiagnosticsWithHotness;
   Conf.RemarksFilename = CGOpts.OptRecordFile;
   Conf.RemarksPasses = CGOpts.OptRecordPasses;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5737,6 +5737,9 @@
 
 let Flags = [CC1Option, NoDriverOption] in {
 
+def llvm_verify_each : Flag<["-"], "llvm-verify-each">,
+  HelpText<"Run the LLVM verifier after every LLVM pass">,
+  MarshallingInfoFlag>;
 def disable_llvm_verifier : Flag<["-"], "disable-llvm-verifier">,
   HelpText<"Don't run the LLVM IR verifier pass">,
   MarshallingInfoNegativeFlag>;
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -322,6 +322,8 @@
 
 CODEGENOPT(VerifyModule  , 1, 1) ///< Control whether the module should be 
run
  ///< through the LLVM Verifier.
+CODEGENOPT(VerifyEach, 1, 1) ///< Control whether the LLVM verifier
+ ///< should run after every pass.
 
 CODEGENOPT(StackRealignment  , 1, 0) ///< Control whether to force stack
  ///< realignment.


Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -259,7 +259,8 @@
   ModuleAnalysisManager MAM;
 
   PassInstrumentationCallbacks PIC;
-  

[PATCH] D153236: [NFC] Fix potential dereferencing of nullptr.

2023-06-21 Thread Sindhu Chittireddy via Phabricator via cfe-commits
schittir added inline comments.



Comment at: clang/lib/Parse/ParseStmt.cpp:886-887
 TopLevelCase = Case;
-  else
+  else {
+assert(DeepestParsedCaseStmt && "DeepestParsedCaseStmt cannot be 
null");
 Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, Case.get());

aaron.ballman wrote:
> The `assert` doesn't add much value, this seems to be a false positive in the 
> analysis.
> 
> If we have no top-level case statement yet, then we get into the `if` branch 
> on line 884, but that then sets `DeepestParsedCaseStmt` on line 890, and that 
> must be non-null because we verified that `Case` is valid above on line 876. 
> So by the time we get into the `else` branch, `DeepestParsedCaseStmt` must be 
> nonnull.
> 
> I don't think changes are needed here at all; WDYT?
This makes sense. I didn't thoroughly consider the Case.isInvalid() check above 
in my analysis. Thank you for catching this! I removed this change. 


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

https://reviews.llvm.org/D153236

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


[PATCH] D153236: [NFC] Fix potential dereferencing of nullptr.

2023-06-21 Thread Sindhu Chittireddy via Phabricator via cfe-commits
schittir updated this revision to Diff 533386.
schittir marked 3 inline comments as done.
schittir added a comment.

Changes to address comments.


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

https://reviews.llvm.org/D153236

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/lib/Sema/SemaType.cpp


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2708,8 +2708,8 @@
 return QualType();
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
-  if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+  if (const auto *BIT = CurType->getAs()) {
+unsigned NumBits = BIT->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  
PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,9 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(((isSuperReceiver && Loc.isValid()) || receiverTypeInfo) &&
+ "Either the super receiver location needs to be valid or the receiver 
"
+ "needs valid type source information");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : 
SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,


Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2708,8 +2708,8 @@
 return QualType();
   }
   // Only support _BitInt elements with byte-sized power of 2 NumBits.
-  if (CurType->isBitIntType()) {
-unsigned NumBits = CurType->getAs()->getNumBits();
+  if (const auto *BIT = CurType->getAs()) {
+unsigned NumBits = BIT->getNumBits();
 if (!llvm::isPowerOf2_32(NumBits) || NumBits < 8) {
   Diag(AttrLoc, diag::err_attribute_invalid_bitint_vector_type)
   << (NumBits < 8);
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1363,10 +1363,9 @@
 if (!Context.hasSameType(PropertyIvarType, IvarType)) {
   if (isa(PropertyIvarType)
   && isa(IvarType))
-compat =
-  Context.canAssignObjCInterfaces(
-  PropertyIvarType->getAs(),
-  IvarType->getAs());
+compat = Context.canAssignObjCInterfaces(
+PropertyIvarType->castAs(),
+IvarType->castAs());
   else {
 compat = (CheckAssignmentConstraints(PropertyIvarLoc, PropertyIvarType,
  IvarType)
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -2438,6 +2438,9 @@
   if (!ReceiverType.isNull())
 receiverTypeInfo = Context.getTrivialTypeSourceInfo(ReceiverType);
 
+  assert(((isSuperReceiver && Loc.isValid()) || receiverTypeInfo) &&
+ "Either the super receiver location needs to be valid or the receiver "
+ "needs valid type source information");
   return BuildClassMessage(receiverTypeInfo, ReceiverType,
   /*SuperLoc=*/isSuperReceiver ? Loc : SourceLocation(),
Sel, Method, Loc, Loc, Loc, Args,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-21 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

In D152785#4438544 , @MaskRay wrote:

> CodeView is the default debug info format for COFF. Perhaps `-gsplit-dwarf` 
> should be rejected when the default `-gcodeview` is used, with a test.

IIRC some users do generate both CodeView and DWARF debug info at the same 
time, for some use cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152785

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


[PATCH] D152986: [clang] Allow 'nomerge' attribute for function pointers

2023-06-21 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

In D152986#4426256 , @eddyz87 wrote:

> In D152986#4425736 , @rnk wrote:
>
>> The purpose of the attribute is really limited to preserving source location 
>> information on instructions, and this isn't really a supported usage.
>
> But it would prevent merging, right? I understand that using this attribute 
> might block some legitimate optimizations.
> Or do you mean that it is a "best effort" thing and not all transformations 
> might honor it?

Essentially, yes.

Anyway, I think this looks good, please apply the doc update.




Comment at: clang/include/clang/Basic/AttrDocs.td:551-555
 calls to the specified function from merging. It has no effect on indirect 
 calls.
+
+``nomerge`` attribute can be specified for pointers to functions, all
+calls done through such pointer would be protected from merging.

eddyz87 wrote:
> rnk wrote:
> > This statement of the attribute having "no effect on indirect calls" is 
> > slightly confusing now that we talk about function pointers immediately 
> > afterward. Can you please rework this a bit, and clarify that when applied 
> > to function pointers, the attribute only takes effect when the call target 
> > is directly the variable which carries the attribute? For example, this has 
> > no effect:
> > ```
> > void (*fp)() __attribute__((nomerge));
> > void callit() {
> >   auto tmp = fp;
> >   tmp();
> >   (*fp)(); // I think TargetDecl will be null in the code, tell me if I'm 
> > wrong
> > }
> > ```
> I can make it more elaborate, would the text as below be fine?
> Regarding TargetDecl value it is not null both times:
> - `(VarDecl 'tmp' (ImplicitCastExpr (DeclRefExpr (Var fp`
> - `(VarDecl 'fp')`
> 
> ---
> 
> ``nomerge`` attribute can also be used as function attribute to prevent all 
> calls to the specified function from merging. It has no effect on indirect 
> calls to such functions. For example:
> 
> .. code-block:: c++
> 
>   [[clang::nomerge]] void foo(int) {}
>   
>   void bar(int x) {
> auto *ptr = foo;
> if (x) foo(1); else foo(2); // will not be merged
> if (x) ptr(1); else ptr(2); // indirect call, can be merged
>   }
> 
> ``nomerge`` attribute can also be used for pointers to functions to
> prevent calls through such pointer from merging. In such case the
> effect applies only to a specific function pointer. For example:
> 
> .. code-block:: c++
> 
>   [[clang::nomerge]] void (*foo)(int);
>   
>   void bar(int x) {
> auto *ptr = foo;
> if (x) foo(1); else foo(2); // will not be merged
> if (x) ptr(1); else ptr(2); // 'ptr' has no 'nomerge' attribute,
> // can be merged
>   }
> 
Looks good to me, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152986

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


[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

2023-06-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

> In this case function `fileno` returns -1 because of failure, but this is not 
> indicated in a `NoteTag`. This is a correct result, only the note is missing. 
> This problem can be solved if a note is displayed on every branch ("case") of 
> the standard C functions. But this leads to many notes at un-interesting 
> places. If the note is displayed only at "interesting" values another 
> difficulty shows up: The note disappears from places where it should be shown 
> because the "interestingness" is not set, for example at conditions of `if` 
> statement. So the solution may require more work. This case with function 
> `fileno` occurs 13 times in all the tested projects.

Extra notes along the path are fine as long as their `isPrunable()` flag is 
correctly set. It's perfectly ok to say //"(15) Hey btw we've just ran over 
this statement and here's what we assume it did"// in a section of a path 
that's already displayed to the user. Even if you say that on every line, it's 
probably ok.

But if the note causes a new nested stack frame to be displayed (which was 
otherwise pruned from the report), there better be a good reason for this, 
because this can easily increase the complexity of the bug report by a factor 
of 100.

It's definitely a requirement for a non-alpha checker to make sure that there 
are enough non-prunable pieces in the report for the user to understand the 
report. A lot of our existing on-by-default checkers (and even some `core` 
checkers) don't really hold up to this expectation (looking at you null 
dereference), but almost every time they don't, that's a false positive. If you 
need to pass `-analyzer-config prune-paths=false` in order to see a key step in 
the bug report, that's a false positive even if your path simulation was 
perfect.

So if any of these notes are essential to understanding any bug report (by this 
checker or by another checker, eg. like `getenv()` returning null is essential 
for null dereference checker), there needs to be a way for the note tag to 
learn that (eg., the `getenv()` should be able to ask whether the return value 
is being tracked by `trackExpressionValue()`) and if so, the note has to be 
marked as unprunable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152436

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


[PATCH] D129635: [OpenMP] Update the default version of OpenMP to 5.1

2023-06-21 Thread H. Vetinari via Phabricator via cfe-commits
h-vetinari added a comment.

Does https://clang.llvm.org/docs/OpenMPSupport.html need an update? It still 
says "Clang fully supports OpenMP 4.5" (with many 5.0/5.1 features marked as 
"worked on" / "unclaimed"), which would make it unusual to put the default on a 
version that's (according to that status page) only ~30% implemented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129635

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


[PATCH] D153293: [clang][WebAssembly] support wasm32-wasi shared libraries

2023-06-21 Thread Joel Dice via Phabricator via cfe-commits
dicej updated this revision to Diff 533375.
dicej added a comment.

remove config.py changes per review feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153293

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain.c
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -99,13 +99,6 @@
 return Reloc::Static;
   }
 
-  if (!TT.isOSEmscripten()) {
-// Relocation modes other than static are currently implemented in a way
-// that only works for Emscripten, so disable them if we aren't targeting
-// Emscripten.
-return Reloc::Static;
-  }
-
   return *RM;
 }
 
Index: clang/test/Driver/wasm-toolchain.c
===
--- clang/test/Driver/wasm-toolchain.c
+++ clang/test/Driver/wasm-toolchain.c
@@ -33,6 +33,20 @@
 // LINK_KNOWN: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
 // LINK_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
 
+// -shared should be passed through to `wasm-ld` and not include crt1.o with a known OS.
+
+// RUN: %clang -### -shared --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_KNOWN_SHARED %s
+// LINK_KNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_KNOWN_SHARED: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi" "-shared" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
+// -shared should be passed through to `wasm-ld` and not include crt1.o with an unknown OS.
+
+// RUN: %clang -### -shared --target=wasm32-unknown-unknown --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_UNKNOWN_SHARED %s
+// LINK_UNKNOWN_SHARED: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_UNKNOWN_SHARED: wasm-ld{{.*}}" "-shared" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+
 // A basic C link command-line with optimization with known OS.
 
 // RUN: %clang -### -O2 --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
@@ -46,6 +60,18 @@
 // RUN:   | FileCheck -check-prefix=COMPILE %s
 // COMPILE: "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" "/foo/include"
 
+// -fPIC should work on a known OS
+
+// RUN: %clang -### -fPIC --target=wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=COMPILE_KNOWN_PIC %s
+// COMPILE_KNOWN_PIC: "-cc1" {{.*}} "-mrelocation-model" "pic" "-pic-level" "2" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi" "-internal-isystem" "/foo/include"
+
+// -fPIC should work on an unknown OS
+
+// RUN: %clang -### -fPIC --target=wasm32-unknown-unknown --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=COMPILE_UNKNOWN_PIC %s
+// COMPILE_UNKNOWN_PIC: "-cc1" {{.*}} "-mrelocation-model" "pic" "-pic-level" "2"
+
 // Thread-related command line tests.
 
 // '-pthread' sets +atomics, +bulk-memory, +mutable-globals, +sign-ext, and --shared-memory
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -101,13 +101,16 @@
   << CM << A->getOption().getName();
 }
   }
-  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles))
+  if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles, options::OPT_shared))
 CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(Crt1)));
   if (Entry) {
 CmdArgs.push_back(Args.MakeArgString("--entry"));
 CmdArgs.push_back(Args.MakeArgString(Entry));
   }
 
+  if (Args.hasArg(options::OPT_shared))
+  CmdArgs.push_back(Args.MakeArgString("-shared"));
+
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -701,6 +701,12 @@
 
 WebAssembly Support
 ^^^
+- Shared library support (and PIC code generation) for WebAssembly is no longer
+  limited to the Emscripten target OS and now works with other targets such as
+  wasm32-wasi.  Note that the `format
+  `_
+  is not yet stable and may change between LLVM versions.  Also, WASI does not
+  yet have facilities to load dynamic libraries.
 
 AVR Support
 ^^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

2023-06-21 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Uh-oh, looks like I'm not paying nearly enough attention to this discussion 
(sorry about that!!)

I'm somewhat skeptical of the decision made in D151225 
 because the entire reason I originally 
implemented `StdCLibraryFunctions` was to deal with false positives I was 
seeing. It was really valuable even without the bug-finding part. So I really 
wish we could find some way to keep bug-finding and modeling separate.

I haven't read the entire discussion though, I need to catch up 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152436

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


[PATCH] D153462: [Headers][doc] Add various arith/logical intrinsic descriptions to avx2intrin.h

2023-06-21 Thread Paul Robinson via Phabricator via cfe-commits
probinson created this revision.
probinson added reviewers: pengfei, RKSimon, goldstein.w.n, craig.topper.
Herald added a project: All.
probinson requested review of this revision.

https://reviews.llvm.org/D153462

Files:
  clang/lib/Headers/avx2intrin.h

Index: clang/lib/Headers/avx2intrin.h
===
--- clang/lib/Headers/avx2intrin.h
+++ clang/lib/Headers/avx2intrin.h
@@ -19,22 +19,112 @@
 #define __DEFAULT_FN_ATTRS128 __attribute__((__always_inline__, __nodebug__, __target__("avx2"), __min_vector_width__(128)))
 
 /* SSE4 Multiple Packed Sums of Absolute Difference.  */
+/// Computes sixteen sum of absolute difference (SAD) operations on sets of
+///four unsigned 8-bit integers from the 256-bit integer vectors \a X and
+///\a Y.
+///
+///Eight SAD results are computed using the lower half of the input
+///vectors, and another eight using the upper half. These 16-bit values
+///are returned in the lower and upper halves of the 256-bit result,
+///respectively.
+///
+///A single SAD operation selects four bytes from \a X and four bytes from
+///\a Y as input. It computes the differences between each \a X byte and
+///the corresponding \a Y byte, takes the absolute value of each
+///difference, and sums these four values to form one 16-bit result. The
+///intrinsic computes 16 of these results with different sets of input
+///bytes.
+///
+///For each set of eight results, the SAD operations use the same four
+///bytes from \a Y; the starting bit position for these four bytes is
+///specified by \a M[1:0] times 32. The eight operations use successive
+///sets of four bytes from \a X; the starting bit position for the first
+///set of four bytes is specified by \a M[2] times 32. These bit positions
+///are all relative to the 128-bit lane for each set of eight operations.
+///
+/// \code{.operation}
+/// r := 0
+/// FOR i := 0 TO 1
+///   j := i*3
+///   Ybase := M[j+1:j]*32 + i*128
+///   Xbase := M[j+2]*32 + i*128
+///   FOR k := 0 TO 3
+/// temp0 := ABS(X[Xbase+7:Xbase] - Y[Ybase+7:Ybase])
+/// temp1 := ABS(X[Xbase+15:Xbase+8] - Y[Ybase+15:Ybase+8])
+/// temp2 := ABS(X[Xbase+23:Xbase+16] - Y[Ybase+23:Ybase+16])
+/// temp3 := ABS(X[Xbase+31:Xbase+24] - Y[Ybase+31:Ybase+24])
+/// result[r+15:r] := temp0 + temp1 + temp2 + temp3
+/// Xbase := Xbase + 8
+/// r := r + 16
+///   ENDFOR
+/// ENDFOR
+/// \endcode
+///
+/// \headerfile 
+///
+/// \code
+/// __m256i _mm256_mpsadbw_epu8(__m256i X, __m256i Y, const int M);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c VMPSADBW instruction.
+///
+/// \param X
+///A 256-bit integer vector containing one of the inputs.
+/// \param Y
+///A 256-bit integer vector containing one of the inputs.
+/// \param M
+/// An unsigned immediate value specifying the starting positions of the
+/// bytes to operate on.
+/// \returns A 256-bit vector of [16 x i16] containing the result.
 #define _mm256_mpsadbw_epu8(X, Y, M) \
   ((__m256i)__builtin_ia32_mpsadbw256((__v32qi)(__m256i)(X), \
   (__v32qi)(__m256i)(Y), (int)(M)))
 
+/// Computes the absolute value of each signed byte in the 256-bit integer
+///vector \a __a and returns each value in the corresponding byte of
+///the result.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VPABSB instruction.
+///
+/// \param __a
+///A 256-bit integer vector.
+/// \returns A 256-bit integer vector containing the result.
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_abs_epi8(__m256i __a)
 {
 return (__m256i)__builtin_elementwise_abs((__v32qs)__a);
 }
 
+/// Computes the absolute value of each signed 16-bit element in the 256-bit
+///vector of [16 x i16] in \a __a and returns each value in the
+///corresponding element of the result.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VPABSW instruction.
+///
+/// \param __a
+///A 256-bit vector of [16 x i16].
+/// \returns A 256-bit vector of [16 x i16] containing the result.
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_abs_epi16(__m256i __a)
 {
 return (__m256i)__builtin_elementwise_abs((__v16hi)__a);
 }
 
+/// Computes the absolute value of each signed 32-bit element in the 256-bit
+///vector of [8 x i32] in \a __a and returns each value in the
+///corresponding element of the result.
+///
+/// \headerfile 
+///
+/// This intrinsic corresponds to the \c VPABSD instruction.
+///
+/// \param __a
+///A 256-bit vector of [8 x i32].
+/// \returns A 256-bit vector of [8 x i32] containing the result.
 static __inline__ __m256i __DEFAULT_FN_ATTRS256
 _mm256_abs_epi32(__m256i __a)
 {
@@ -345,24 +435,88 @@
   ((__m256i)__builtin_ia32_palignr256((__v32qi)(__m256i)(a), \
   (__v32qi)(__m256i)(b), (n)))
 
+/// Computes the bitwise AND of the 

[PATCH] D152946: [C++20][Modules] Implement P2615R1 revised export diagnostics.

2023-06-21 Thread Iain Sandoe via Phabricator via cfe-commits
iains added a comment.

In D152946#4431974 , @h-vetinari 
wrote:

> Mark P2615  as implemented in 
> https://github.com/llvm/llvm-project/blame/main/clang/www/cxx_status.html?



In D152946#4431974 , @h-vetinari 
wrote:

> Mark P2615  as implemented in 
> https://github.com/llvm/llvm-project/blame/main/clang/www/cxx_status.html?

I did not do that yet since this patch only implements the removal of unnamed 
export diagnostics.  There will be a follow-on patch shortly that implements 
the addition of the export specialisation diagnostics and then we can claim 
P2615R1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152946

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


[PATCH] D153059: [-Wunsafe-buffer-usage] Group parameter fix-its

2023-06-21 Thread Ziqing Luo via Phabricator via cfe-commits
ziqingluo-90 added inline comments.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2077
+
   for (const auto &[VD, Fixables] : FixablesForUnsafeVars.byVar) {
 FixItsForVariable[VD] =

NoQ wrote:
> There's a bug in variable naming: `FixablesForUnsafeVars`actually contains 
> fixables for *all* variables. Rashmi correctly renames it in D150489 to 
> reflect its actual behavior. So IIUC you're generating fixits for all 
> variables here, which is probably not what you want.
Thanks for pointing out the incorrectness of the variable name.   Actually, 
whether a variable is unsafe is irrelevant to this function.  The function only 
cares about all variables that need fixes.  So I think the incorrect name of 
the variable does not affect the functionality of my changes here.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2278-2281
+  if (isParameterOf(Impl.first, D))
+ParmsNeedFix.insert(Impl.first);
+  if (isParameterOf(Impl.second, D))
+ParmsNeedFix.insert(Impl.second);

NoQ wrote:
> Does this really go in both directions? Shouldn't it be just one direction?
> 
> ```lang=c++
> void foo(int *p) {
>   int *q = new int[10];
>   p = q;
>   q[5] = 7;
> }
> ```
> In this case `q` is an unsafe local buffer, but `p` is a (mostly) safe 
> single-object pointer that doesn't need fixing, despite implication from `q` 
> to `p`. Of course this example is somewhat unrealistic (people don't usually 
> overwrite their parameters), but it is also the only situation where the 
> other direction matters.
You are right!  It should be one-direction.



Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:2293
+//  search of connected components.
+if (!ParmsNeedFix.empty()) {
+  auto First = ParmsNeedFix.begin(), Last = First;

NoQ wrote:
> What about the situation where params aren't seen as unsafe yet, but they're 
> discovered to be unsafe later? Eg.
> ```
> void foo(int *p, int *q) {
>   int *p2 = p;
>   p2[5] = 7;
>   int *q2 = q;
>   q2[5] = 7;
> }
> ```
> Will we notice that `p` and `q` need to be fixed simultaneously?
> 
> ---
> 
> I suspect that the problem of parameter grouping can't be solved by 
> pre-populating strategy implications between all parameters. It'll either 
> cause you to implicate all parameters (even the ones that will never need 
> fixing), or cause you to miss connections between parameters that do need 
> fixing (as the connection is discovered later).
> 
> Connection between parameters needs to be added *after* the implications 
> algorithm has finished. And once it's there (might be already there if I 
> missed something), then this part of code probably becomes unnecessary.
Yes.  They will be noticed.  Here is why:

The code here first forms a ring over `p` and  `q` in the assignment (directed) 
graph:  
```
p <--> q
```
Then the two initialization statements (implemented in [[ 
https://reviews.llvm.org/D150489 | D150489 ]]) add two more edges to the graph:
```
p2 --> p <--> q <-- q2
```
The algorithm later searches the graph starting from unsafe variables `p2` and 
`q2`, respectively,  Starting from `p2`, the algorithm discovers that `p2` and 
`p`, as well as `p` and `q` depend on each other.  Starting from `q2`,  the 
algorithm discovers that `q2` and `q`, as well as `p` and `q` depend on each 
other.   The dependency graph is sort of undirected. So eventually, the four 
variables `p2`, `p`, `q`, `q2` are in the same group.




Comment at: 
clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-multi-parm-span.cpp:13-15
+   expected-note{{change type of 'p' to 'std::span' to preserve bounds 
information, and change 'q' and 'a' to 'std::span' to propagate bounds 
information between them}} \
+   expected-note{{change type of 'q' to 'std::span' to preserve bounds 
information, and change 'p' and 'a' to 'std::span' to propagate bounds 
information between them}} \
+   expected-note{{change type of 'a' to 'std::span' to preserve bounds 
information, and change 'p' and 'q' to 'std::span' to propagate bounds 
information between them}}

NoQ wrote:
> QoL thing: //"to propagate bounds information between them"// isn't the 
> correct reason in this case. Bounds information doesn't propagate between `p` 
> and `q` and `a`. Each of them carries its own, completely unrelated bounds 
> information.
> 
> We need to come up with a different reason. Or it might be better to combine 
> the warnings into one warning here, so that we didn't need a reason:
> ```
> warning: 'p', 'q' and 'a' are unsafe pointers used for buffer access
> note: change type of 'p', 'q' and 'a' to 'std::span' to preserve bounds 
> information
> ```
> 
> This gets a bit worse when parameters are implicated indirectly, but it still 
> mostly works fine, for example:
> ```
> void foo(int *p, int *q) {
>   int *p2 = p;
>   p2[5] = 7;
>   int *q2 

[PATCH] D153458: [clang-tidy] Model noexcept more properly in bugprone-exception-escape

2023-06-21 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp, isuckatcs, JonasToth, 
baloghadamsoftware.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

During call stack analysis skip called noexcept functions
as they wont throw exceptions, they will crash.
Check will emit warnings for those functions separately.

Fixes: #43667, #49151, #51596, #54668, #54956


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153458

Files:
  clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -152,7 +152,7 @@
 }
 
 // FIXME: In this case 'a' is convertible to the handler and should be caught
-// but in reality it's thrown. Note that clang doesn't report a warning for 
+// but in reality it's thrown. Note that clang doesn't report a warning for
 // this either.
 void throw_catch_multi_ptr_5() noexcept {
   try {
@@ -249,7 +249,7 @@
 void throw_derived_catch_base_ptr_c() noexcept {
   try {
 derived d;
-throw  
+throw 
   } catch(const base *) {
   }
 }
@@ -259,7 +259,7 @@
   try {
 derived d;
 const derived *p = 
-throw p; 
+throw p;
   } catch(base *) {
   }
 }
@@ -282,7 +282,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private' which should not throw exceptions
   try {
 B b;
-throw b; 
+throw b;
   } catch(A) {
   }
 }
@@ -291,7 +291,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private_ptr' which should not throw exceptions
   try {
 B b;
-throw  
+throw 
   } catch(A *) {
   }
 }
@@ -300,7 +300,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected' which should not throw exceptions
   try {
 C c;
-throw c; 
+throw c;
   } catch(A) {
   }
 }
@@ -309,7 +309,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected_ptr' which should not throw exceptions
   try {
 C c;
-throw  
+throw 
   } catch(A *) {
   }
 }
@@ -318,7 +318,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous' which should not throw exceptions
   try {
 E e;
-throw e; 
+throw e;
   } catch(A) {
   }
 }
@@ -327,7 +327,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous_ptr' which should not throw exceptions
   try {
 E e;
-throw e; 
+throw e;
   } catch(A) {
   }
 }
@@ -703,3 +703,22 @@
   throw 1;
   return 0;
 }
+
+// The following function all incorrectly throw exceptions, *but* calling them
+// should not yield a warning because they are marked as noexcept.
+
+void test_basic_no_throw() noexcept { throw 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'test_basic_no_throw' which should not throw exceptions
+
+void test_basic_throw() noexcept(false) { throw 42; }
+
+void only_calls_non_throwing() noexcept {
+  test_basic_no_throw();
+}
+
+void calls_non_and_throwing() noexcept {
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'calls_non_and_throwing' which should not throw exceptions
+  test_basic_no_throw();
+  test_basic_throw();
+}
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -254,7 +254,8 @@
 
 - Improved :doc:`bugprone-exception-escape
   ` to not emit warnings for
-  forward declarations of functions.
+  forward declarations of functions and additionally modified it to skip
+  ``noexcept`` functions during call stack analysis.
 
 - Fixed :doc:`bugprone-exception-escape`
   for coroutines where previously a warning would be emitted with coroutines
Index: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
===
--- clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
+++ clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -316,6 +316,27 @@
 }
 } // namespace
 
+static bool cannotThrow(const FunctionDecl *Func) {
+  const auto *FunProto = Func->getType()->getAs();
+  if (!FunProto)
+return false;
+
+  

[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

2023-06-21 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

I haven't looked closely at this, but from a vague/quick reading, it sounds 
like this is creating annotations on certain type DINodes, but then moving them 
around to different types? It'd be nice if we could avoid that/create them in 
the right place in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143967

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


[PATCH] D153293: [clang][WebAssembly] support wasm32-wasi shared libraries

2023-06-21 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added inline comments.



Comment at: llvm/utils/lit/lit/llvm/config.py:347
+return triple
+
 m = re.match(r"(\w+)-(\w+)-(\w+)", triple)

dicej wrote:
> sbc100 wrote:
> > Are the the changes to this file meant to be part of this CL?
> The `check-clang` target doesn't work at all if the target "triple" is a 
> double, e.g.:
> 
> ```
> llvm-lit: 
> /Users/dicej/p/wasi-sdk/src/llvm-project/llvm/utils/lit/lit/llvm/config.py:459:
>  note: using clang: /Users/dicej/p/wasi-sdk/build/llvm/bin/clang
> llvm-lit: 
> /Users/dicej/p/wasi-sdk/src/llvm-project/llvm/utils/lit/lit/llvm/config.py:324:
>  fatal: Could not turn 'wasm32-wasi' into Itanium ABI triple
> ```
> 
> @sunfish had been using this patch locally to work around the issue, so I 
> figured I'd include the patch here so it stops being a stumbling block.  
> Perhaps there's a better way to address it?
Can you split this out into a separate change.. its seem unrelated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153293

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


[PATCH] D153205: [clang-format] Add new block type ListInit

2023-06-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment.

In D153205#4437966 , @MyDeveloperDay 
wrote:

> My only concern is that this changes behaviour without someone making the 
> conscious decision to make that change, now as much as I can't understand why 
> someone would want to put the `}` onto the same line, it will only take on 
> person to say "hey I liked it like that" to complain and call it a 
> regression, or that we somehow change the defaults (which people always 
> complain about even through we don't normally do that)
>
> In the past our solution to this problem is to have an option with a "Leave" 
> setting to sort of say, leave it as it currently is. In this case it feels 
> like a bug, looks like a bug and is probably a bug, but I'd be interested in 
> @owenpan and @HazardyKnusperkeks's view

The github issue basically says all there's to say, if it those constructs 
should be formatted like function calls, then that's what should happen. Or we 
should adapt the documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153205

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


[PATCH] D153418: Adding iconv support to CharSetConverter class

2023-06-21 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

This doesn't really address the concerns from 
https://discourse.llvm.org/t/rfc-adding-a-charset-converter-to-the-llvm-support-library/69795/17
 about consistency.  It's bad if different hosts support a different set of 
charsets/charset names, and it's really bad if a given charset name has 
different meanings on different hosts.

Can we use the existing conversion utilities in LLVM for UTF-16/UTF-32?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153418

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


[PATCH] D153417: New CharSetConverter wrapper class for ConverterEBCDIC

2023-06-21 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

As I mentioned at 
https://discourse.llvm.org/t/rfc-enabling-fexec-charset-support-to-llvm-and-clang-reposting/71512
 , I think SimplifyLibCalls needs to be aware of encodings.  To make that work, 
this probably needs to be somewhere in llvm/ , not clang/ .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153417

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


[PATCH] D146148: [clang] Add a namespace for interesting identifiers.

2023-06-21 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Thanks, LGTM!


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

https://reviews.llvm.org/D146148

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


[PATCH] D153293: [clang][WebAssembly] support wasm32-wasi shared libraries

2023-06-21 Thread Joel Dice via Phabricator via cfe-commits
dicej added a comment.

In D153293#4437877 , @sbc100 wrote:

> In D153293#4437644 , @dicej wrote:
>
>> In D153293#4436629 , @sbc100 wrote:
>>
>>> I'm happy to merge it, but perhaps we should get @sunfish to lg too.
>>>
>>> Also, should we remove the `-experimental-pic` linker flag, or are you OK 
>>> with that warning from the linker?   If yes, I wonder if we should do that 
>>> as part of this CL or a followup?
>>
>> Do `emscripten` users see that warning, too?  I'd like to match whatever 
>> they see (or don't see).
>
> No, the emscripten driver (emcc) currently adds `--experimental-pic` to the 
> linker flags to avoid this warning.  We could gladly remove that if it 
> becomes the default.

OK; I'd say let's leave the warning there since this feature so new for 
non-emscripten targets.  We can come back and remove it later on once things 
have settled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153293

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


[PATCH] D146148: [clang] Add a namespace for interesting identifiers.

2023-06-21 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 533360.
zahiraam marked 3 inline comments as done.

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

https://reviews.llvm.org/D146148

Files:
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Basic/TokenKinds.h
  clang/lib/Basic/Builtins.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Sema/SemaDecl.cpp

Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6763,14 +6763,22 @@
   if (IdentifierInfo *II = NewTD->getIdentifier())
 if (!NewTD->isInvalidDecl() &&
 NewTD->getDeclContext()->getRedeclContext()->isTranslationUnit()) {
-  if (II->isStr("FILE"))
+  switch (II->getInterestingIdentifierID()) {
+  case tok::InterestingIdentifierKind::FILE:
 Context.setFILEDecl(NewTD);
-  else if (II->isStr("jmp_buf"))
+break;
+  case tok::InterestingIdentifierKind::jmp_buf:
 Context.setjmp_bufDecl(NewTD);
-  else if (II->isStr("sigjmp_buf"))
+break;
+  case tok::InterestingIdentifierKind::sigjmp_buf:
 Context.setsigjmp_bufDecl(NewTD);
-  else if (II->isStr("ucontext_t"))
+break;
+  case tok::InterestingIdentifierKind::ucontext_t:
 Context.setucontext_tDecl(NewTD);
+break;
+  default:
+break;
+  }
 }
 
   return NewTD;
Index: clang/lib/Basic/IdentifierTable.cpp
===
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -279,6 +279,16 @@
   Table.get(Name).setObjCKeywordID(ObjCID);
 }
 
+static void AddInterestingIdentifier(StringRef Name,
+ tok::InterestingIdentifierKind BTID,
+ IdentifierTable ) {
+  // Don't add 'not_interesting' identifier.
+  if (BTID != tok::not_interesting) {
+IdentifierInfo  = Table.get(Name, tok::identifier);
+Info.setInterestingIdentifierID(BTID);
+  }
+}
+
 /// AddKeywords - Add all keywords to the symbol table.
 ///
 void IdentifierTable::AddKeywords(const LangOptions ) {
@@ -295,6 +305,9 @@
 #define OBJC_AT_KEYWORD(NAME)  \
   if (LangOpts.ObjC)   \
 AddObjCKeyword(StringRef(#NAME), tok::objc_##NAME, *this);
+#define INTERESTING_IDENTIFIER(NAME)   \
+  AddInterestingIdentifier(StringRef(#NAME), tok::##NAME, *this);
+
 #define TESTING_KEYWORD(NAME, FLAGS)
 #include "clang/Basic/TokenKinds.def"
 
Index: clang/lib/Basic/Builtins.cpp
===
--- clang/lib/Basic/Builtins.cpp
+++ clang/lib/Basic/Builtins.cpp
@@ -151,7 +151,7 @@
   unsigned ID = NameIt->second->getBuiltinID();
   if (ID != Builtin::NotBuiltin && isPredefinedLibFunction(ID) &&
   isInStdNamespace(ID) == InStdNamespace) {
-Table.get(Name).setBuiltinID(Builtin::NotBuiltin);
+NameIt->second->clearBuiltinID();
   }
 }
   }
Index: clang/include/clang/Basic/TokenKinds.h
===
--- clang/include/clang/Basic/TokenKinds.h
+++ clang/include/clang/Basic/TokenKinds.h
@@ -44,6 +44,14 @@
   NUM_OBJC_KEYWORDS
 };
 
+/// Provides a namespace for interesting identifers such as float_t and
+/// double_t.
+enum InterestingIdentifierKind {
+#define INTERESTING_IDENTIFIER(X) X,
+#include "clang/Basic/TokenKinds.def"
+  NUM_INTERESTING_IDENTIFIERS
+};
+
 /// Defines the possible values of an on-off-switch (C99 6.10.6p2).
 enum OnOffSwitch {
   OOS_ON, OOS_OFF, OOS_DEFAULT
Index: clang/include/clang/Basic/TokenKinds.def
===
--- clang/include/clang/Basic/TokenKinds.def
+++ clang/include/clang/Basic/TokenKinds.def
@@ -85,6 +85,9 @@
 #ifndef PRAGMA_ANNOTATION
 #define PRAGMA_ANNOTATION(X) ANNOTATION(X)
 #endif
+#ifndef INTERESTING_IDENTIFIER
+#define INTERESTING_IDENTIFIER(X)
+#endif
 
 //===--===//
 // Preprocessor keywords.
@@ -794,6 +797,15 @@
 OBJC_AT_KEYWORD(import)
 OBJC_AT_KEYWORD(available)
 
+//===--===//
+// Interesting idenitifiers.
+//===--===//
+INTERESTING_IDENTIFIER(not_interesting)
+INTERESTING_IDENTIFIER(FILE)
+INTERESTING_IDENTIFIER(jmp_buf)
+INTERESTING_IDENTIFIER(sigjmp_buf)
+INTERESTING_IDENTIFIER(ucontext_t)
+
 // TODO: What to do about context-sensitive keywords like:
 //   bycopy/byref/in/inout/oneway/out?
 
@@ -974,3 +986,4 @@
 #undef TOK
 #undef C99_KEYWORD
 #undef C2X_KEYWORD
+#undef INTERESTING_IDENTIFIER
Index: clang/include/clang/Basic/IdentifierTable.h

[PATCH] D146148: [clang] Add a namespace for interesting identifiers.

2023-06-21 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Yes, this is very close, thank you.




Comment at: clang/include/clang/Basic/TokenKinds.def:805
+INTERESTING_IDENTIFIER(float_t)
+INTERESTING_IDENTIFIER(double_t)
+INTERESTING_IDENTIFIER(FILE)

I think it would be cleaner if you added these two as interesting identifiers 
in your follow-up patch.



Comment at: clang/lib/Sema/SemaLookup.cpp:950
+  return true;
+}
 // In C++ and OpenCL (spec v1.2 s6.9.f), we don't have any predefined

The two changes in this file shouldn't be necessary, since interesting 
identifiers should be disjoint from builtins.


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

https://reviews.llvm.org/D146148

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


[PATCH] D153418: Adding iconv support to CharSetConverter class

2023-06-21 Thread Abhina Sree via Phabricator via cfe-commits
abhina.sreeskantharajan marked an inline comment as done.
abhina.sreeskantharajan added inline comments.



Comment at: clang/lib/Basic/CMakeLists.txt:62
+if(Iconv_FOUND AND NOT Iconv_IS_BUILT_IN)
+  set(system_libs ${system_libs} ${Iconv_LIBRARIES})
+endif()

michaelplatings wrote:
> This doesn't look like an idomatic way to link a library. Could you use [[ 
> https://cmake.org/cmake/help/latest/command/target_link_libraries.html | 
> target_link_libraries ]] instead?
Thanks, I've made this change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153418

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


[PATCH] D153418: Adding iconv support to CharSetConverter class

2023-06-21 Thread Abhina Sree via Phabricator via cfe-commits
abhina.sreeskantharajan updated this revision to Diff 533352.
abhina.sreeskantharajan added a reviewer: michaelplatings.
abhina.sreeskantharajan added a comment.

Use target_link_libraries instead


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153418

Files:
  clang/include/clang/Basic/CharSet.h
  clang/include/clang/Config/config.h.cmake
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/CharSet.cpp
  clang/unittests/Basic/CharSetTest.cpp

Index: clang/unittests/Basic/CharSetTest.cpp
===
--- clang/unittests/Basic/CharSetTest.cpp
+++ clang/unittests/Basic/CharSetTest.cpp
@@ -40,6 +40,29 @@
 // String with Cyrillic character ya.
 static const char CyrillicUTF[] = "\xd0\xaf";
 
+// String "Earth地球".
+// ISO-2022-JP: Sequence ESC $ B (\x1B\x24\x42) switches to JIS X 0208-1983, and
+// sequence ESC ( B (\x1B\x28\x42) switches back to ASCII.
+// IBM-939: Byte 0x0E shifts from single byte to double byte, and 0x0F shifts
+// back.
+static const char EarthUTF[] = "\x45\x61\x72\x74\x68\xe5\x9c\xb0\xe7\x90\x83";
+// Identical to above, except the final character (球) has its last byte taken
+// away from it.
+static const char EarthUTFBroken[] = "\x45\x61\x72\x74\x68\xe5\x9c\xb0\xe7\x90";
+static const char EarthISO2022[] =
+"\x45\x61\x72\x74\x68\x1B\x24\x42\x43\x4F\x35\x65\x1B\x28\x42";
+static const char EarthISO2022ShiftBack[] =
+"\x45\x61\x72\x74\x68\x1B\x24\x42\x43\x4F\x35\x65";
+static const char EarthIBM939[] =
+"\xc5\x81\x99\xa3\x88\x0e\x45\xc2\x48\xdb\x0f";
+static const char ShiftBackOnly[] = "\x1B\x28\x42";
+
+// String "地球".
+static const char EarthKanjiOnlyUTF[] = "\xe5\x9c\xb0\xe7\x90\x83";
+static const char EarthKanjiOnlyISO2022[] =
+"\x1B\x24\x42\x43\x4F\x35\x65\x1b\x28\x42";
+static const char EarthKanjiOnlyIBM939[] = "\x0e\x45\xc2\x48\xdb\x0f";
+
 TEST(CharSet, FromUTF8) {
   // Hello string.
   StringRef Src(HelloA);
@@ -98,4 +121,154 @@
   EXPECT_STREQ(AccentUTF, static_cast(Dst).c_str());
 }
 
+TEST(CharSet, RoundTrip) {
+  ErrorOr ConvToUTF16 =
+  CharSetConverter::create("IBM-1047", "UTF-16");
+  // Stop test if conversion is not supported (no underlying iconv support).
+  if (!ConvToUTF16) {
+ASSERT_EQ(ConvToUTF16.getError(),
+  std::make_error_code(std::errc::invalid_argument));
+return;
+  }
+  ErrorOr ConvToUTF32 =
+  CharSetConverter::create("UTF-16", "UTF-32");
+  // Stop test if conversion is not supported (no underlying iconv support).
+  if (!ConvToUTF32) {
+ASSERT_EQ(ConvToUTF32.getError(),
+  std::make_error_code(std::errc::invalid_argument));
+return;
+  }
+  ErrorOr ConvToEBCDIC =
+  CharSetConverter::create("UTF-32", "IBM-1047");
+  // Stop test if conversion is not supported (no underlying iconv support).
+  if (!ConvToEBCDIC) {
+ASSERT_EQ(ConvToEBCDIC.getError(),
+  std::make_error_code(std::errc::invalid_argument));
+return;
+  }
+
+  // Setup source string.
+  char SrcStr[256];
+  for (size_t I = 0; I < 256; ++I)
+SrcStr[I] = (I + 1) % 256;
+
+  SmallString<99> Dst1Str, Dst2Str, Dst3Str;
+
+  std::error_code EC = ConvToUTF16->convert(StringRef(SrcStr), Dst1Str, true);
+  EXPECT_TRUE(!EC);
+  EC = ConvToUTF32->convert(Dst1Str, Dst2Str, true);
+  EXPECT_TRUE(!EC);
+  EC = ConvToEBCDIC->convert(Dst2Str, Dst3Str, true);
+  EXPECT_TRUE(!EC);
+  EXPECT_STREQ(SrcStr, static_cast(Dst3Str).c_str());
+}
+
+TEST(CharSet, ShiftState2022) {
+  // Earth string.
+  StringRef Src(EarthUTF);
+  SmallString<64> Dst;
+
+  ErrorOr ConvTo2022 =
+  CharSetConverter::create("UTF-8", "ISO-2022-JP");
+  // Stop test if conversion is not supported (no underlying iconv support).
+  if (!ConvTo2022) {
+ASSERT_EQ(ConvTo2022.getError(),
+  std::make_error_code(std::errc::invalid_argument));
+return;
+  }
+
+  // Check that the string is properly converted.
+  std::error_code EC = ConvTo2022->convert(Src, Dst, true);
+  EXPECT_TRUE(!EC);
+  EXPECT_STREQ(EarthISO2022, static_cast(Dst).c_str());
+}
+
+TEST(CharSet, ShiftState2022Flush) {
+  StringRef Src0(EarthUTFBroken);
+  StringRef Src1(EarthKanjiOnlyUTF);
+  SmallString<64> Dst0;
+  SmallString<64> Dst1;
+  ErrorOr ConvTo2022Flush =
+  CharSetConverter::create("UTF-8", "ISO-2022-JP");
+  if (!ConvTo2022Flush) {
+ASSERT_EQ(ConvTo2022Flush.getError(),
+  std::make_error_code(std::errc::invalid_argument));
+return;
+  }
+
+  // This should emit an error; there is a malformed multibyte character in the
+  // input string.
+  std::error_code EC0 = ConvTo2022Flush->convert(Src0, Dst0, true);
+  EXPECT_TRUE(EC0);
+  std::error_code EC1 = ConvTo2022Flush->flush();
+  EXPECT_TRUE(!EC1);
+  std::error_code EC2 = ConvTo2022Flush->convert(Src1, Dst1, true);
+  EXPECT_TRUE(!EC2);
+  EXPECT_STREQ(EarthKanjiOnlyISO2022, static_cast(Dst1).c_str());
+}
+
+TEST(CharSet, 

[PATCH] D143241: [Clang] Reset FP options before function instantiations

2023-06-21 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/Parse/ParseTemplate.cpp:1736
+  // point of the template definition.
+  Actions.resetFPOptions(LPT.FPO);
+

Ah, is this bug specific to the MSVC-compatible template logic?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143241

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


[PATCH] D153419: Enable fexec-charset option

2023-06-21 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6864
+  CmdArgs.push_back("-fexec-charset");
+  CmdArgs.push_back(Args.MakeArgString(Triple.getSystemCharset()));
   if (Arg *execCharset = Args.getLastArg(options::OPT_fexec_charset_EQ)) {

The default should be in the "else"?  It's confusing to pass two -fexec-charset 
arguments, even if it works.



Comment at: clang/lib/Lex/LiteralSupport.cpp:2145
+memcpy(Cp, CpConv.data(), ResultLength);
+ResultPtr = Cp + CpConv.size();
+  }

Can the code be refactored to put this repeated sequence into a helper function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153419

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


[PATCH] D153267: [clang][Diagnostics] Provide parameter source range to arity-mismatch notes

2023-06-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment.

I think this will be a good addition because it underscores the salient part of 
why the function isn't viable.

Would it be possible for patches like this to have both examples of before and 
after this patch in their commit message please? That'll help expose why 
patches like this are desirable. Happy to LGTM afterwards.




Comment at: clang/test/Misc/diag-func-call-ranges.cpp:13
+void arity_mismatch() {
+  (void)func(2, 4);
+}

Why are you casting to void?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153267

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


[PATCH] D143241: [Clang] Reset FP options before function instantiations

2023-06-21 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143241

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


[PATCH] D143241: [Clang] Reset FP options before function instantiations

2023-06-21 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143241

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


[PATCH] D153359: [clang][Diagnostics] Fix distant source ranges in bad-conversion notes

2023-06-21 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment.

I like this patch, thanks for working on it 

In D153359#4436873 , @hazohelet wrote:

> Consider the following code. (I added another parameter to the original code 
> so that the covered range becomes clearer)
>
>   void func(int aa, int bb);
>   
>   
>   void test() { func(1, "two"); }
>
> BEFORE this patch:
>
>   source:4:15: error: no matching function for call to 'func'
>   4 | void test() { func(1, "two"); }
> |   ^~~~
>   source:1:6: note: candidate function not viable: no known conversion from 
> 'const char[4]' to 'int' for 2nd argument
>   1 | void func(int aa, int bb);
> |  ^
>   2 |
>   3 |
>   4 | void test() { func(1, "two"); }
> |   ~
>
> AFTER this patch:
>
>   source:4:15: error: no matching function for call to 'func'
>   4 | void test() { func(1, "two"); }
> |   ^~~~
>   source:1:6: note: candidate function not viable: no known conversion from 
> 'const char[4]' to 'int' for 2nd argument
>   1 | void func(int aa, int bb);
> |  ^~~

Having this in the commit message would be great, thanks!




Comment at: clang/lib/Sema/SemaOverload.cpp:10821
 
+// FIXME: No test case for this. Can we remove this block?
 if (FromQs.hasUnaligned() != ToQs.hasUnaligned()) {

Please don't commit until this is resolved (either tests added or it's removed).


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

https://reviews.llvm.org/D153359

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


[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

CodeView is the default debug info format for COFF. Perhaps `-gsplit-dwarf` 
should be rejected when the default `-gcodeview` is used, with a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152785

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


[PATCH] D152785: [COFF] Support -gsplit-dwarf for COFF on Windows

2023-06-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision.
MaskRay added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Driver/Options.td:1167
 // GCC style -dumpdir. We intentionally don't implement the less useful 
-dumpbase{,-ext}.
-def dumpdir : Separate<["-"], "dumpdir">, Flags<[CC1Option]>,
+def dumpdir : Separate<["-"], "dumpdir">, Flags<[CC1Option, CoreOption]>,
   MetaVarName<"">,

HaohaiWen wrote:
> MaskRay wrote:
> > This exposes `-dumpdir ` to clang-cl which may not be useful.
> clang -gdwarf -gsplit-dwarf foo.cpp -o foo.exe -### will be expanded to
> clang -cc1 ... "-dumpdir" "foo.exe-"  "-split-dwarf-file" 
> "foo.exe-foo.dwo"
> 
> dwo name will be prefixed by dumpdir if it's specified.
> 
> This dumpdir is required for clang-cl to have same dwo filename behavior as 
> clang.
CoreOption allows `-dumpdir` to be used with clang-cl. If you do this, I expect 
a driver test case.

clang-cl passes `-dumpdir` to clang -cc1. `-dumpdir` has the CC1Option flag, so 
`-cc1` accepts the option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152785

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


[PATCH] D146148: [clang] Add a namespace for interesting identifiers.

2023-06-21 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam marked an inline comment as done.
zahiraam added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:6784
+  if (II->getInterestingIdentifierID() != 0)
+NewTD->addAttr(AvailableOnlyInDefaultEvalMethodAttr::Create(Context));
 }

rjmccall wrote:
> zahiraam wrote:
> > rjmccall wrote:
> > > Please switch over the interesting identifiers here; we don't want to 
> > > assume this feature is only used for these two names.
> > > 
> > > In fact, should we go ahead and immediately apply it to the four 
> > > identifiers above this?  That would be nice, because then we could 
> > > actually do this in two patches: one patch that does the refactor to 
> > > track interesting identifiers but doesn't cause any functionality changes 
> > > and a second, very small patch that just introduces the new special 
> > > treatment for `float_t` and `double_t`.
> > > Please switch over the interesting identifiers here; we don't want to 
> > > assume this feature is only used for these two names.
> > > 
> > > In fact, should we go ahead and immediately apply it to the four 
> > > identifiers above this?  That would be nice, because then we could 
> > > actually do this in two patches: one patch that does the refactor to 
> > > track interesting identifiers but doesn't cause any functionality changes 
> > > and a second, very small patch that just introduces the new special 
> > > treatment for `float_t` and `double_t`.
> > 
> > Are you saying that "FILE", "jmp_buf"," sigjmp_buf" and "ucontext_t" are 
> > also interesting identifiers? If yes, they should be added to the list of 
> > interesting identifiers in TokenKinds.def?
> Right.  The basic idea of interesting identifiers is to replace these sorts 
> of identifier comparisons in performance-critical code.  So your first patch 
> would *only* add those four identifiers as interesting identifiers, handling 
> them here by registering the `typedef` with the ASTContext like the code is 
> already doing.  Then you'd make a follow-up patch that adds `float_t` and 
> `double_t` and handles them here by implicitly adding your new attribute.
> Right.  The basic idea of interesting identifiers is to replace these sorts 
> of identifier comparisons in performance-critical code.  So your first patch 
> would *only* add those four identifiers as interesting identifiers, handling 
> them here by registering the `typedef` with the ASTContext like the code is 
> already doing.  Then you'd make a follow-up patch that adds `float_t` and 
> `double_t` and handles them here by implicitly adding your new attribute.

I think that does it?


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

https://reviews.llvm.org/D146148

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


[PATCH] D146148: Float_t and double_t types shouldn't be modified by #pragma clang fp eval_method

2023-06-21 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 54.

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

https://reviews.llvm.org/D146148

Files:
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Basic/TokenKinds.h
  clang/lib/Basic/Builtins.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaLookup.cpp

Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -320,10 +320,11 @@
   // Compiler builtins are always visible, regardless of where they end
   // up being declared.
   if (IdentifierInfo *Id = NameInfo.getName().getAsIdentifierInfo()) {
-if (unsigned BuiltinID = Id->getBuiltinID()) {
-  if (!getSema().Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
-AllowHidden = true;
-}
+if (!Id->getInterestingIdentifierID())
+  if (unsigned BuiltinID = Id->getBuiltinID()) {
+if (!getSema().Context.BuiltinInfo.isPredefinedLibFunction(BuiltinID))
+  AllowHidden = true;
+  }
   }
 }
 
@@ -943,6 +944,10 @@
 
   // If this is a builtin on this (or all) targets, create the decl.
   if (unsigned BuiltinID = II->getBuiltinID()) {
+if (unsigned ID = II->getInterestingIdentifierID()) {
+  // Don't think we should LazyCreate the interesting identifier?
+  return true;
+}
 // In C++ and OpenCL (spec v1.2 s6.9.f), we don't have any predefined
 // library functions like 'malloc'. Instead, we'll just error.
 if ((getLangOpts().CPlusPlus || getLangOpts().OpenCL) &&
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -6763,14 +6763,22 @@
   if (IdentifierInfo *II = NewTD->getIdentifier())
 if (!NewTD->isInvalidDecl() &&
 NewTD->getDeclContext()->getRedeclContext()->isTranslationUnit()) {
-  if (II->isStr("FILE"))
+  switch (II->getInterestingIdentifierID()) {
+  case tok::InterestingIdentifierKind::FILE:
 Context.setFILEDecl(NewTD);
-  else if (II->isStr("jmp_buf"))
+break;
+  case tok::InterestingIdentifierKind::jmp_buf:
 Context.setjmp_bufDecl(NewTD);
-  else if (II->isStr("sigjmp_buf"))
+break;
+  case tok::InterestingIdentifierKind::sigjmp_buf:
 Context.setsigjmp_bufDecl(NewTD);
-  else if (II->isStr("ucontext_t"))
+break;
+  case tok::InterestingIdentifierKind::ucontext_t:
 Context.setucontext_tDecl(NewTD);
+break;
+  default:
+break;
+  }
 }
 
   return NewTD;
Index: clang/lib/Basic/IdentifierTable.cpp
===
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -279,6 +279,16 @@
   Table.get(Name).setObjCKeywordID(ObjCID);
 }
 
+static void AddInterestingIdentifier(StringRef Name,
+ tok::InterestingIdentifierKind BTID,
+ IdentifierTable ) {
+  // Don't add 'not_interesting' identifier.
+  if (BTID != tok::not_interesting) {
+IdentifierInfo  = Table.get(Name, tok::identifier);
+Info.setInterestingIdentifierID(BTID);
+  }
+}
+
 /// AddKeywords - Add all keywords to the symbol table.
 ///
 void IdentifierTable::AddKeywords(const LangOptions ) {
@@ -295,6 +305,9 @@
 #define OBJC_AT_KEYWORD(NAME)  \
   if (LangOpts.ObjC)   \
 AddObjCKeyword(StringRef(#NAME), tok::objc_##NAME, *this);
+#define INTERESTING_IDENTIFIER(NAME)   \
+  AddInterestingIdentifier(StringRef(#NAME), tok::##NAME, *this);
+
 #define TESTING_KEYWORD(NAME, FLAGS)
 #include "clang/Basic/TokenKinds.def"
 
Index: clang/lib/Basic/Builtins.cpp
===
--- clang/lib/Basic/Builtins.cpp
+++ clang/lib/Basic/Builtins.cpp
@@ -151,7 +151,7 @@
   unsigned ID = NameIt->second->getBuiltinID();
   if (ID != Builtin::NotBuiltin && isPredefinedLibFunction(ID) &&
   isInStdNamespace(ID) == InStdNamespace) {
-Table.get(Name).setBuiltinID(Builtin::NotBuiltin);
+NameIt->second->clearBuiltinID();
   }
 }
   }
Index: clang/include/clang/Basic/TokenKinds.h
===
--- clang/include/clang/Basic/TokenKinds.h
+++ clang/include/clang/Basic/TokenKinds.h
@@ -44,6 +44,14 @@
   NUM_OBJC_KEYWORDS
 };
 
+/// Provides a namespace for interesting identifers such as float_t and
+/// double_t.
+enum InterestingIdentifierKind {
+#define INTERESTING_IDENTIFIER(X) X,
+#include "clang/Basic/TokenKinds.def"
+  NUM_INTERESTING_IDENTIFIERS
+};
+
 /// Defines the possible values of an on-off-switch (C99 6.10.6p2).
 enum OnOffSwitch {
   

[PATCH] D153379: Remove -flang-experimental-exec flag

2023-06-21 Thread Brad Richardson via Phabricator via cfe-commits
everythingfunctional added a comment.

In D153379#4438419 , @PeteSteinfeld 
wrote:

> All builds and tests correctly and looks good.
>
> @everythingfunctional, can you please post on Discourse about this change 
> when you merge?  Since we've discussed this with the community, I think we 
> should let them know that this is happening.

Absolutely. Thanks for approving.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153379

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


[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

2023-06-21 Thread Di Mo via Phabricator via cfe-commits
modimo updated this revision to Diff 533326.
modimo added a comment.

Address feedback. Allow empty string to unset the flag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152741

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGVTables.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCXX/type-metadata-unknown-vtable-visibility-filepaths.cpp
  clang/test/Driver/funknown-vtable-visibility-filepaths.c
  llvm/include/llvm/IR/GlobalObject.h
  llvm/lib/IR/Metadata.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/ThinLTO/X86/devirt.ll

Index: llvm/test/ThinLTO/X86/devirt.ll
===
--- llvm/test/ThinLTO/X86/devirt.ll
+++ llvm/test/ThinLTO/X86/devirt.ll
@@ -42,9 +42,11 @@
 ; RUN:   -r=%t2.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t2.o,_ZN1C1fEi,p \
 ; RUN:   -r=%t2.o,_ZN1D1mEi,p \
+; RUN:   -r=%t2.o,_ZN1E1nEi,p \
 ; RUN:   -r=%t2.o,_ZTV1B,px \
 ; RUN:   -r=%t2.o,_ZTV1C,px \
-; RUN:   -r=%t2.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN:   -r=%t2.o,_ZTV1D,px \
+; RUN:   -r=%t2.o,_ZTV1F,px 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; Check that we're able to prevent specific function from being
@@ -58,9 +60,11 @@
 ; RUN:   -r=%t2.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t2.o,_ZN1C1fEi,p \
 ; RUN:   -r=%t2.o,_ZN1D1mEi,p \
+; RUN:   -r=%t2.o,_ZN1E1nEi,p \
 ; RUN:   -r=%t2.o,_ZTV1B,px \
 ; RUN:   -r=%t2.o,_ZTV1C,px \
-; RUN:   -r=%t2.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=SKIP
+; RUN:   -r=%t2.o,_ZTV1D,px \
+; RUN:   -r=%t2.o,_ZTV1F,px 2>&1 | FileCheck %s --check-prefix=SKIP
 
 ; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \
 ; RUN:   -whole-program-visibility \
@@ -70,16 +74,20 @@
 ; RUN:   -r=%t.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t.o,_ZN1C1fEi,p \
 ; RUN:   -r=%t.o,_ZN1D1mEi,p \
+; RUN:   -r=%t.o,_ZN1E1nEi,p \
 ; RUN:   -r=%t.o,_ZTV1B, \
 ; RUN:   -r=%t.o,_ZTV1C, \
 ; RUN:   -r=%t.o,_ZTV1D, \
+; RUN:   -r=%t.o,_ZTV1F, \
 ; RUN:   -r=%t.o,_ZN1A1nEi, \
 ; RUN:   -r=%t.o,_ZN1B1fEi, \
 ; RUN:   -r=%t.o,_ZN1C1fEi, \
 ; RUN:   -r=%t.o,_ZN1D1mEi, \
+; RUN:   -r=%t.o,_ZN1E1nEi, \
 ; RUN:   -r=%t.o,_ZTV1B,px \
 ; RUN:   -r=%t.o,_ZTV1C,px \
-; RUN:   -r=%t.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=REMARK --dump-input=fail
+; RUN:   -r=%t.o,_ZTV1D,px \
+; RUN:   -r=%t.o,_ZTV1F,px 2>&1 | FileCheck %s --check-prefix=REMARK --dump-input=fail
 ; RUN: llvm-dis %t3.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
@@ -95,13 +103,18 @@
 %struct.C = type { %struct.A }
 %struct.D = type { ptr }
 
+%struct.E = type { ptr }
+%struct.F = type { %struct.E }
+
 @_ZTV1B = constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr undef, ptr @_ZN1B1fEi, ptr @_ZN1A1nEi] }, !type !0, !type !1
 @_ZTV1C = constant { [4 x ptr] } { [4 x ptr] [ptr null, ptr undef, ptr @_ZN1C1fEi, ptr @_ZN1A1nEi] }, !type !0, !type !2
 @_ZTV1D = constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr undef, ptr @_ZN1D1mEi] }, !type !3
 
+@_ZTV1F = constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr undef, ptr @_ZN1E1nEi] }, !type !5, !vcall_visibility !6
+
 
 ; CHECK-IR-LABEL: define i32 @test
-define i32 @test(ptr %obj, ptr %obj2, i32 %a) {
+define i32 @test(ptr %obj, ptr %obj2, ptr %obj3, i32 %a) {
 entry:
   %vtable = load ptr, ptr %obj
   %p = call i1 @llvm.type.test(ptr %vtable, metadata !"_ZTS1A")
@@ -114,7 +127,7 @@
   ; Ensure !prof and !callees metadata for indirect call promotion removed.
   ; CHECK-IR-NOT: prof
   ; CHECK-IR-NOT: callees
-  %call = tail call i32 %fptr1(ptr nonnull %obj, i32 %a), !prof !5, !callees !6
+  %call = tail call i32 %fptr1(ptr nonnull %obj, i32 %a), !prof !7, !callees !8
 
   %fptr22 = load ptr, ptr %vtable, align 8
 
@@ -131,7 +144,18 @@
   ; Check that the call was devirtualized.
   ; CHECK-IR: %call4 = tail call i32 @_ZN1D1mEi
   %call4 = tail call i32 %fptr33(ptr nonnull %obj2, i32 %call3)
-  ret i32 %call4
+
+  %vtable1 = load ptr, ptr %obj3
+  %p3 = call i1 @llvm.type.test(ptr %vtable1, metadata !"_ZTS1E")
+  call void @llvm.assume(i1 %p3)
+  %fptrptr1 = getelementptr ptr, ptr %vtable1, i32 0
+  %fptr44 = load ptr, ptr %fptrptr1, align 8
+
+  ; Check that the call was not devirtualized because of "unknown"
+  ; vcall_visibility.
+  ; CHECK-IR: %call5 = tail call i32 %fptr44
+  %call5 = tail call i32 %fptr44(ptr nonnull %obj, i32 %call4)
+  ret i32 %call5
 }
 ; CHECK-IR-LABEL: ret i32
 ; CHECK-IR-LABEL: }
@@ -155,6 +179,10 @@
ret i32 0;
 }
 
+define i32 @_ZN1E1nEi(ptr %this, i32 %a) #0 {
+   ret i32 0;
+}
+
 ; Make sure we don't inline or otherwise optimize out the direct calls.
 attributes #0 = { noinline optnone }
 
@@ -163,5 +191,7 @@
 !2 = !{i64 16, !"_ZTS1C"}
 !3 

[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

2023-06-21 Thread Di Mo via Phabricator via cfe-commits
modimo added a comment.

In D152741#4438007 , @tejohnson wrote:

> Ok what I missed is that you don't want to apply this to entire TUs, but 
> rather just some paths that are header files, which may be included in many 
> source files. So in your above example, you really only need to apply to the 
> path of third-party/include/boost.h - is that correct?

Yep!

> That would mark class A, and therefore anything derived from it won't get 
> devirtualized. I guess in your example above, you are trying to prevent the 
> devirtualization in a.cpp since there are hidden overrides (class C) in 
> boost.a native objects.

Exactly, we saw this scenario causing issues when enabling WPD.

> The example included with the patch applies the option to the source file of 
> the test case, and therefore its entire TU. It would be helpful to have a 
> test case structured like your example above, where the file path is just 
> that of the header.

Makes sense and yeah the test case is confusing. Changed it to apply to just 
the header file.

> Maybe a better option name is something like 
> -funknown-vtable-visibility-filepaths= ? It seems a bit more descriptive.

Sure, changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152741

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


[PATCH] D153379: Remove -flang-experimental-exec flag

2023-06-21 Thread Pete Steinfeld via Phabricator via cfe-commits
PeteSteinfeld accepted this revision.
PeteSteinfeld added a comment.

All builds and tests correctly and looks good.

@everythingfunctional, can you please post on Discourse about this change when 
you merge?  Since we've discussed this with the community, I think we should 
let them know that this is happening.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153379

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


[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

2023-06-21 Thread Hongtao Yu via Phabricator via cfe-commits
hoy added a comment.

In D144006#4437804 , @dzhidzhoev 
wrote:

> In D144006#4435677 , @hoy wrote:
>
>> Hello, I'm seeing a build failure that seems related to this patch. I'm 
>> seeing the patch has been relanded and reverted a couple times and not sure 
>> where it is right now. Can you please confirm if the failure is related and 
>> fixed? Thanks.
>
> Hello! Currently, the commit is reverted (in commit 
> 6bea8331f9e09ba94a225c65becd4224a1a473af 
> ). Does 
> the build failure still occur with it?
> Could you provide a reproducing code for the crash?

Thanks for the information. As of today the build failure does not exist. It 
only happened yesterday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144006

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


[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

2023-06-21 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy added a comment.

In D152436#4437822 , @steakhal wrote:

> In D152436#4437692 , @donat.nagy 
> wrote:
>
>> Personally I think it's completely acceptable if the analyzer sometimes 
>> emits bug reports that are true positives but lack a message [...]
>
> I must admit that I'm in the other camp.

To clarify my position the "sometimes" in my comment should've been "rarely" or 
something closer to that. I don't want to release confusing garbage, I'm just 
trying to optimize sum(helpfulness of checkers) and I feel that the last few 
steps towards perfect helpfulness are often disproportionately expensive. If a 
code contains two bugs, then two rough but understandable warnings are more 
valuable than one well-polished friendly warning + one completely missed bug 
(because we didn't have time to write a checker for it).

---

@balazske:

Do I understand it correctly that the "create note that's shown when the return 
value is marked as interesting" (the option 2 that I suggested) adds / will add 
clarifying notes to the fileno/fstat error reports and the ftell issue 
?
 Did you see any real-life examples where this option 2 does not provide useful 
notes? If not, then I think we can accept that option 2 is a sufficient 
solution for these issues.

In addition to this, there are these issues noted by @Szelethus:

In D152436#4408199 , @Szelethus wrote:

> In D152436#4405558 , @balazske 
> wrote:
>
>> link 
>> 
>> The function `open` is not modeled in `StdCLibraryFunctionsChecker`, it 
>> should not return less than -1 but this information is not included now.
>> link 
>> 
>> `socket` can not return less than -1 but this function is not modeled 
>> currently.
>
> These should be a rather painless fix, right?
>
> [...]
>
>> link 
>> 
>> `fwrite` with 0 buffer and 0 size should not be an error, this is not 
>> checked now.
>
> Some discussion for that: D140387#inline-1360054 
> . There is a FIXME in the 
> code for it -- not sure how common this specific issue is, but we did stumble 
> on it in an open source project... how painful would it be to fix this?

If it's not too difficult, then please upload a new version of this patch that 
implements "option 2" (i.e. produces notes when the return value of the std 
library function is marked as interesting) + handles the three requests of 
@Szelethus. I hope that you can do this and then this review can be concluded 
quickly.

If there are significant obstacles (or I misunderstood the situation), then 
please reply and discuss the next steps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152436

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


[PATCH] D134351: AArch64: add definitions for recent Apple CPUs

2023-06-21 Thread Moth via Phabricator via cfe-commits
tigermoth added a comment.

Now that AES, SHA2, SHA3 and SM4 features have been removed from armv8.6-a, 
these Apple CPUs can be changed again from 8.5-a to more correctly specified as 
armv8.6-a

https://reviews.llvm.org/D141606?id=488968


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

https://reviews.llvm.org/D134351

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


[PATCH] D153338: [clang-format] vim integration: Mention python3 variant of bindings

2023-06-21 Thread Jannik Silvanus via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9741ac5b3b3e: [clang-format] vim integration: Mention 
python3 variant of bindings (authored by jsilvanus).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153338

Files:
  clang/docs/ClangFormat.rst


Index: clang/docs/ClangFormat.rst
===
--- clang/docs/ClangFormat.rst
+++ clang/docs/ClangFormat.rst
@@ -145,8 +145,13 @@
 
 .. code-block:: vim
 
-  map  :pyf /clang-format.py
-  imap  :pyf /clang-format.py
+  if has('python')
+map  :pyf /clang-format.py
+imap  :pyf /clang-format.py
+  elseif has('python3')
+map  :py3f /clang-format.py
+imap  :py3f /clang-format.py
+  endif
 
 The first line enables :program:`clang-format` for NORMAL and VISUAL mode, the
 second line adds support for INSERT mode. Change "C-K" to another binding if


Index: clang/docs/ClangFormat.rst
===
--- clang/docs/ClangFormat.rst
+++ clang/docs/ClangFormat.rst
@@ -145,8 +145,13 @@
 
 .. code-block:: vim
 
-  map  :pyf /clang-format.py
-  imap  :pyf /clang-format.py
+  if has('python')
+map  :pyf /clang-format.py
+imap  :pyf /clang-format.py
+  elseif has('python3')
+map  :py3f /clang-format.py
+imap  :py3f /clang-format.py
+  endif
 
 The first line enables :program:`clang-format` for NORMAL and VISUAL mode, the
 second line adds support for INSERT mode. Change "C-K" to another binding if
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 9741ac5 - [clang-format] vim integration: Mention python3 variant of bindings

2023-06-21 Thread Jannik Silvanus via cfe-commits

Author: Jannik Silvanus
Date: 2023-06-21T17:18:36+02:00
New Revision: 9741ac5b3b3e7942e3332e37af829e58e5e2bb82

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

LOG: [clang-format] vim integration: Mention python3 variant of bindings

The instructions in the documentation only mentioned how to include
bindings for clang-format into vim using python2. Add the instructions
for python3 which were already present in the source comments.

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

Change-Id: I25fdbd36f0c7e745061908be8e26f68cb31c7dd5

Added: 


Modified: 
clang/docs/ClangFormat.rst

Removed: 




diff  --git a/clang/docs/ClangFormat.rst b/clang/docs/ClangFormat.rst
index 98350a04ccda4..129d5f101a0c4 100644
--- a/clang/docs/ClangFormat.rst
+++ b/clang/docs/ClangFormat.rst
@@ -145,8 +145,13 @@ This can be integrated by adding the following to your 
`.vimrc`:
 
 .. code-block:: vim
 
-  map  :pyf /clang-format.py
-  imap  :pyf /clang-format.py
+  if has('python')
+map  :pyf /clang-format.py
+imap  :pyf /clang-format.py
+  elseif has('python3')
+map  :py3f /clang-format.py
+imap  :py3f /clang-format.py
+  endif
 
 The first line enables :program:`clang-format` for NORMAL and VISUAL mode, the
 second line adds support for INSERT mode. Change "C-K" to another binding if



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


[PATCH] D152741: [WPD] implement -fskip-vtable-filepaths

2023-06-21 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.

In D152741#4422462 , @modimo wrote:

> In D152741#4421067 , @tejohnson 
> wrote:
>
>> In D152741#4419366 , @modimo wrote:
>>
>>> In D152741#4419324 , @tejohnson 
>>> wrote:
>>>
 In D152741#4419265 , @modimo 
 wrote:

> In D152741#4418831 , @tejohnson 
> wrote:
>
>> I think I understand the motivation, but not sure I agree this is the 
>> right approach - can you simply not pass -flto-unit and 
>> -fwhole-program-vtables for these files?
>
> For our third-party libraries, they're pre-built into native files by GCC 
> so that's unfortunately not an option.

 I'm confused - how would you pass this new option then? I was assuming you 
 were passing this option to some LLVM built files at the interface of 
 those libraries. In which case not passing -flto-unit and 
 -fwhole-program-visibility should have a similar effect (suppress the type 
 metadata).
>>>
>>> Oh I see, I misunderstood. Yes this is being passed to LLVM built files. We 
>>> want to avoid manual allowlists/blocklists because code changes make it 
>>> less flexible and scalable than an automatic option.
>>
>> It seems like you need allowlists or blocklists in either case - either it 
>> is passed as a regex via the option proposed here, or the build system 
>> modifies the options for that set of files.
>>
>>> This can also be pretty tricky to do correctly since we can get type 
>>> metadata from multiple TUs and all of them would need to be opted out for 
>>> WPD to not kick in.
>>
>> But clang is presumably compiling a single TU at a time, so your regex needs 
>> to cover them all anyway? I'm not sure I understand the distinction between 
>> doing something like -fskip-vtable-filepaths=third-party/.* vs something 
>> like applying -funknown-vtable-visibility to each third-party/*.cc compile.
>
> The blocklists need to be enforced on internal files that interact with 
> native libraries and those live in many different areas:
>
>   ; /third-party/include/boost.h
>   
>   class A {}
>   
>   ; /internal-source/a.cpp
>   #include "boost.h"
>   
>   class B : public A
>   
>   ; /third-party/lib/boost.a
>   #include "boost.h"
>   
>   class C : public A
>
> That being said, this is something the build system can detect and mark.
>
>> I really think the logic for which files to apply this option to belongs in 
>> the build system, not in the clang driver - just like any other clang 
>> option. It isn't clear to me why this particular option should be applied 
>> based on a file regex.
>
> The big advantage of doing this in the FE is that we know which types are 
> actually coming from the native headers. Blocking all types in the TU is 
> overly conservative and also less stable as header changes can effectively 
> turn on/off unrelated large chunks of WPD.

Ok what I missed is that you don't want to apply this to entire TUs, but rather 
just some paths that are header files, which may be included in many source 
files. So in your above example, you really only need to apply to the path of 
third-party/include/boost.h - is that correct? That would mark class A, and 
therefore anything derived from it won't get devirtualized. I guess in your 
example above, you are trying to prevent the devirtualization in a.cpp since 
there are hidden overrides (class C) in boost.a native objects.

The example included with the patch applies the option to the source file of 
the test case, and therefore its entire TU. It would be helpful to have a test 
case structured like your example above, where the file path is just that of 
the header.

Maybe a better option name is something like 
-funknown-vtable-visibility-filepaths= ? It seems a bit more descriptive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152741

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


[PATCH] D152975: [clang-format] Allow break after return keyword

2023-06-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:1484
   LLVMStyle.PenaltyBreakFirstLessLess = 120;
+  LLVMStyle.PenaltyBreakReturn = 100;
   LLVMStyle.PenaltyBreakString = 1000;

is 100 the current penalty?



Comment at: clang/unittests/Format/FormatTest.cpp:22187
+   "  
HereAVeryLongLineThatWillBeFormattedOnMultipleLineAnd"
+   "ShouldNotBeConsiderAsInline;\n"
"});",

the fails the beyonce rule doesn't it, unless you are assuming the test is a 
bug?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152975

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


[PATCH] D153205: [clang-format] Add new block type ListInit

2023-06-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

My only concern is that this changes behaviour without someone making the 
conscious decision to make that change, now as much as I can't understand why 
someone would want to put the `}` onto the same line, it will only take on 
person to say "hey I liked it like that" to complain and call it a regression, 
or that we somehow change the defaults (which people always complain about even 
through we don't normally do that)

In the past our solution to this problem is to have an option with a "Leave" 
setting to sort of say, leave it as it currently is. In this case it feels like 
a bug, looks like a bug and is probably a bug, but I'd be interested in 
@owenpan and @HazardyKnusperkeks's view


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153205

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


[PATCH] D153430: Warn on invalid Arm or AArch64 baremetal target triple

2023-06-21 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings created this revision.
michaelplatings added reviewers: peter.smith, DavidSpickett.
Herald added subscribers: abidh, kristof.beyls.
Herald added a project: All.
michaelplatings requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

A common user mistake is specifying a target of aarch64-none-eabi or
arm-none-elf whereas the correct names are aarch64-none-elf &
arm-none-eabi. Currently if a target of aarch64-none-eabi is specified
then the Generic_ELF toolchain is used, unlike aarch64-none-elf which
will use the BareMetal toolchain. This is unlikely to be intended by the
user so issue a warning that the target is invalid.

The target parser is liberal in what input it accepts so invalid triples
may yield behaviour that's sufficiently close to what the user intended.
Therefore invalid triples were used in many tests. This change updates
those tests to use valid triples.
One test (gnu-mcount.c) relies on the Generic_ELF toolchain behaviour so
change it to explicitly specify aarch64-unknown-none-gnu as the target.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153430

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/lib/Driver/Driver.cpp
  clang/test/CodeGen/aapcs64-align.cpp
  clang/test/CodeGen/aarch64-sign-return-address.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGenCXX/float16-declarations.cpp
  clang/test/Driver/aarch64-cssc.c
  clang/test/Driver/aarch64-d128.c
  clang/test/Driver/aarch64-hbc.c
  clang/test/Driver/aarch64-ite.c
  clang/test/Driver/aarch64-lrcpc3.c
  clang/test/Driver/aarch64-ls64.c
  clang/test/Driver/aarch64-lse128.c
  clang/test/Driver/aarch64-mops.c
  clang/test/Driver/aarch64-mte.c
  clang/test/Driver/aarch64-perfmon.c
  clang/test/Driver/aarch64-predres.c
  clang/test/Driver/aarch64-rand.c
  clang/test/Driver/aarch64-ras.c
  clang/test/Driver/aarch64-rdm.c
  clang/test/Driver/aarch64-ssbs.c
  clang/test/Driver/aarch64-the.c
  clang/test/Driver/arm-matrix-multiply.c
  clang/test/Driver/arm-sb.c
  clang/test/Driver/constructors.c
  clang/test/Driver/unsupported-target-arch.c
  clang/test/Frontend/gnu-mcount.c
  clang/test/Headers/arm-fp16-header.c
  clang/test/Headers/arm-neon-header.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/test/CodeGen/AArch64/andcompare.ll
  llvm/test/CodeGen/AArch64/andorbrcompare.ll
  llvm/test/CodeGen/AArch64/blockaddress.ll
  llvm/test/CodeGen/AArch64/extern-weak.ll
  llvm/test/CodeGen/AArch64/init-array.ll
  llvm/test/CodeGen/AArch64/literal_pools_float.ll
  llvm/test/CodeGen/AArch64/neg-selects.ll
  llvm/test/CodeGen/AArch64/qmovn.ll
  llvm/test/CodeGen/AArch64/select-constant-xor.ll
  llvm/test/MC/AArch64/armv8.9a-clrbhb.s

Index: llvm/test/MC/AArch64/armv8.9a-clrbhb.s
===
--- llvm/test/MC/AArch64/armv8.9a-clrbhb.s
+++ llvm/test/MC/AArch64/armv8.9a-clrbhb.s
@@ -2,37 +2,37 @@
 // Assembly is always permitted for instructions in the hint space.
 
 // Optional, off by default
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8a < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8.8a < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9a < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9.3a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v8.8a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9a < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=+v9.3a < %s | FileCheck %s --check-prefix=HINT_22
 
 // Optional, off by default, doubly disabled
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v8.8a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
-// RUN: llvm-mc -show-encoding -triple aarch64-none-none-eabi -mattr=+v9.3a,-clrbhb < %s | FileCheck %s --check-prefix=HINT_22
+// RUN: llvm-mc -show-encoding -triple aarch64-none-elf -mattr=-clrbhb < %s | FileCheck 

[PATCH] D153418: Adding iconv support to CharSetConverter class

2023-06-21 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings added inline comments.



Comment at: clang/lib/Basic/CMakeLists.txt:62
+if(Iconv_FOUND AND NOT Iconv_IS_BUILT_IN)
+  set(system_libs ${system_libs} ${Iconv_LIBRARIES})
+endif()

This doesn't look like an idomatic way to link a library. Could you use [[ 
https://cmake.org/cmake/help/latest/command/target_link_libraries.html | 
target_link_libraries ]] instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153418

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


[PATCH] D153338: [clang-format] vim integration: Mention python3 variant of bindings

2023-06-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153338

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


[PATCH] D153293: [clang][WebAssembly] support wasm32-wasi shared libraries

2023-06-21 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment.

In D153293#4437644 , @dicej wrote:

> In D153293#4436629 , @sbc100 wrote:
>
>> I'm happy to merge it, but perhaps we should get @sunfish to lg too.
>>
>> Also, should we remove the `-experimental-pic` linker flag, or are you OK 
>> with that warning from the linker?   If yes, I wonder if we should do that 
>> as part of this CL or a followup?
>
> Do `emscripten` users see that warning, too?  I'd like to match whatever they 
> see (or don't see).

No, the emscripten driver (emcc) currently adds `--experimental-pic` to the 
linker flags to avoid this warning.  We could gladly remove that if it becomes 
the default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153293

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


[PATCH] D153423: [clang-tidy] Allow explicit throwing in bugprone-exception-escape for special functions

2023-06-21 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst:29
+and destructors, it is a clear indication of the developer's intention and
+should be respected..
+

Double period.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153423

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


[PATCH] D152436: [clang][analyzer] Move checker alpha.unix.StdCLibraryFunctions out of alpha.

2023-06-21 Thread Balázs Benics via Phabricator via cfe-commits
steakhal resigned from this revision.
steakhal added a comment.

I resign as a reviewer as I'm not deeply connected to this checker, thus I 
won't block it or accept it.
However, my opinion is that a checker should be "released" if they have clear 
diagnostics (which includes that it doesn't flood the user with unimportant 
diagnostics either).
Consequently, if there are features missing to accomplish that, then that thing 
is a blocker.

TBH I never understood why interestingness is not transitive over the `SymExpr` 
dependencies (`symbols_begin/end`).
This was not the only case when it hindered us. Just think of how taint 
propagates 
.

In D152436#4437692 , @donat.nagy 
wrote:

> Personally I think it's completely acceptable if the analyzer sometimes emits 
> bug reports that are true positives but lack a message [...]

I must admit that I'm in the other camp.

> If these issues produce //lots// of issues that are //very// confusing, then 
> we should put the affected functions behind off-by-default flags, finish this 
> review process, and revisit them later when the interestingness system is 
> improved; but based on the available information I don't think that's 
> necessary.

I can agree with this pragmatic approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152436

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


[PATCH] D153424: [clang][ASTImporter] Add import of CXXRewrittenBinaryOperator.

2023-06-21 Thread Balázs Kéri via Phabricator via cfe-commits
balazske created this revision.
Herald added subscribers: steakhal, martong, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
balazske 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/D153424

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/test/ASTMerge/cxx-rewritten-binary-operator/Inputs/rbo.cpp
  clang/test/ASTMerge/cxx-rewritten-binary-operator/Inputs/std-compare.h
  clang/test/ASTMerge/cxx-rewritten-binary-operator/test.cpp

Index: clang/test/ASTMerge/cxx-rewritten-binary-operator/test.cpp
===
--- /dev/null
+++ clang/test/ASTMerge/cxx-rewritten-binary-operator/test.cpp
@@ -0,0 +1,3 @@
+// RUN: %clang_cc1 -std=c++20 -emit-pch -o %t.1.ast %S/Inputs/rbo.cpp
+// RUN: %clang_cc1 -std=c++20 -ast-merge %t.1.ast -fsyntax-only %s 2>&1 | FileCheck --allow-empty %s
+// CHECK-NOT: unsupported AST node
Index: clang/test/ASTMerge/cxx-rewritten-binary-operator/Inputs/std-compare.h
===
--- /dev/null
+++ clang/test/ASTMerge/cxx-rewritten-binary-operator/Inputs/std-compare.h
@@ -0,0 +1,307 @@
+#ifndef STD_COMPARE_H
+#define STD_COMPARE_H
+
+namespace std {
+inline namespace __1 {
+
+// exposition only
+enum class _EqResult : unsigned char {
+  __equal = 0,
+  __equiv = __equal,
+};
+
+enum class _OrdResult : signed char {
+  __less = -1,
+  __greater = 1
+};
+
+enum class _NCmpResult : signed char {
+  __unordered = -127
+};
+
+struct _CmpUnspecifiedType;
+using _CmpUnspecifiedParam = void (_CmpUnspecifiedType::*)();
+
+class partial_ordering {
+  using _ValueT = signed char;
+  explicit constexpr partial_ordering(_EqResult __v) noexcept
+  : __value_(_ValueT(__v)) {}
+  explicit constexpr partial_ordering(_OrdResult __v) noexcept
+  : __value_(_ValueT(__v)) {}
+  explicit constexpr partial_ordering(_NCmpResult __v) noexcept
+  : __value_(_ValueT(__v)) {}
+
+  constexpr bool __is_ordered() const noexcept {
+return __value_ != _ValueT(_NCmpResult::__unordered);
+  }
+
+public:
+  // valid values
+  static const partial_ordering less;
+  static const partial_ordering equivalent;
+  static const partial_ordering greater;
+  static const partial_ordering unordered;
+
+  // comparisons
+  friend constexpr bool operator==(partial_ordering __v, _CmpUnspecifiedParam) noexcept;
+  friend constexpr bool operator!=(partial_ordering __v, _CmpUnspecifiedParam) noexcept;
+  friend constexpr bool operator<(partial_ordering __v, _CmpUnspecifiedParam) noexcept;
+  friend constexpr bool operator<=(partial_ordering __v, _CmpUnspecifiedParam) noexcept;
+  friend constexpr bool operator>(partial_ordering __v, _CmpUnspecifiedParam) noexcept;
+  friend constexpr bool operator>=(partial_ordering __v, _CmpUnspecifiedParam) noexcept;
+  friend constexpr bool operator==(_CmpUnspecifiedParam, partial_ordering __v) noexcept;
+  friend constexpr bool operator!=(_CmpUnspecifiedParam, partial_ordering __v) noexcept;
+  friend constexpr bool operator<(_CmpUnspecifiedParam, partial_ordering __v) noexcept;
+  friend constexpr bool operator<=(_CmpUnspecifiedParam, partial_ordering __v) noexcept;
+  friend constexpr bool operator>(_CmpUnspecifiedParam, partial_ordering __v) noexcept;
+  friend constexpr bool operator>=(_CmpUnspecifiedParam, partial_ordering __v) noexcept;
+
+  friend constexpr partial_ordering operator<=>(partial_ordering __v, _CmpUnspecifiedParam) noexcept;
+  friend constexpr partial_ordering operator<=>(_CmpUnspecifiedParam, partial_ordering __v) noexcept;
+
+  // test helper
+  constexpr bool test_eq(partial_ordering const ) const noexcept {
+return __value_ == other.__value_;
+  }
+
+private:
+  _ValueT __value_;
+};
+
+inline constexpr partial_ordering partial_ordering::less(_OrdResult::__less);
+inline constexpr partial_ordering partial_ordering::equivalent(_EqResult::__equiv);
+inline constexpr partial_ordering partial_ordering::greater(_OrdResult::__greater);
+inline constexpr partial_ordering partial_ordering::unordered(_NCmpResult ::__unordered);
+constexpr bool operator==(partial_ordering __v, _CmpUnspecifiedParam) noexcept {
+  return __v.__is_ordered() && __v.__value_ == 0;
+}
+constexpr bool operator<(partial_ordering __v, _CmpUnspecifiedParam) noexcept {
+  return __v.__is_ordered() && __v.__value_ < 0;
+}
+constexpr bool operator<=(partial_ordering __v, _CmpUnspecifiedParam) noexcept {
+  return __v.__is_ordered() && __v.__value_ <= 0;
+}
+constexpr bool operator>(partial_ordering __v, _CmpUnspecifiedParam) noexcept {
+  return __v.__is_ordered() && __v.__value_ > 0;
+}
+constexpr bool operator>=(partial_ordering __v, _CmpUnspecifiedParam) noexcept {
+  return __v.__is_ordered() && __v.__value_ >= 0;
+}
+constexpr bool operator==(_CmpUnspecifiedParam, partial_ordering __v) 

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

2023-06-21 Thread Vladislav Dzhidzhoev via Phabricator via cfe-commits
dzhidzhoev added a comment.

In D144006#4435677 , @hoy wrote:

> Hello, I'm seeing a build failure that seems related to this patch. I'm 
> seeing the patch has been relanded and reverted a couple times and not sure 
> where it is right now. Can you please confirm if the failure is related and 
> fixed? Thanks.

Hello! Currently, the commit is reverted (in commit 
6bea8331f9e09ba94a225c65becd4224a1a473af 
). Does 
the build failure still occur with it?
Could you provide a reproducing code for the crash?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144006

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


[PATCH] D153423: [clang-tidy] Allow explicit throwing in bugprone-exception-escape for special functions

2023-06-21 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp, isuckatcs, JonasToth, 
baloghadamsoftware.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Functions declared explicitly with noexcept(false) or throw(exception)
will be excluded from the analysis, as even though it is not recommended for
functions like swap, main, move constructors and assignment operators,
and destructors, it is a clear indication of the developer's intention and
should be respected.

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

  https://github.com/llvm/llvm-project/issues/55143


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153423

Files:
  clang-tools-extra/clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/exception-escape.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape-coro.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/exception-escape.cpp
@@ -152,7 +152,7 @@
 }
 
 // FIXME: In this case 'a' is convertible to the handler and should be caught
-// but in reality it's thrown. Note that clang doesn't report a warning for 
+// but in reality it's thrown. Note that clang doesn't report a warning for
 // this either.
 void throw_catch_multi_ptr_5() noexcept {
   try {
@@ -249,7 +249,7 @@
 void throw_derived_catch_base_ptr_c() noexcept {
   try {
 derived d;
-throw  
+throw 
   } catch(const base *) {
   }
 }
@@ -259,7 +259,7 @@
   try {
 derived d;
 const derived *p = 
-throw p; 
+throw p;
   } catch(base *) {
   }
 }
@@ -282,7 +282,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private' which should not throw exceptions
   try {
 B b;
-throw b; 
+throw b;
   } catch(A) {
   }
 }
@@ -291,7 +291,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_private_ptr' which should not throw exceptions
   try {
 B b;
-throw  
+throw 
   } catch(A *) {
   }
 }
@@ -300,7 +300,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected' which should not throw exceptions
   try {
 C c;
-throw c; 
+throw c;
   } catch(A) {
   }
 }
@@ -309,7 +309,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_protected_ptr' which should not throw exceptions
   try {
 C c;
-throw  
+throw 
   } catch(A *) {
   }
 }
@@ -318,7 +318,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous' which should not throw exceptions
   try {
 E e;
-throw e; 
+throw e;
   } catch(A) {
   }
 }
@@ -327,7 +327,7 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: an exception may be thrown in function 'throw_derived_catch_base_ambiguous_ptr' which should not throw exceptions
   try {
 E e;
-throw e; 
+throw e;
   } catch(A) {
   }
 }
@@ -703,3 +703,28 @@
   throw 1;
   return 0;
 }
+
+namespace PR55143 { namespace PR40583 {
+
+struct test_explicit_throw {
+test_explicit_throw() throw(int) { throw 42; }
+test_explicit_throw(const test_explicit_throw&) throw(int) { throw 42; }
+test_explicit_throw(test_explicit_throw&&) throw(int) { throw 42; }
+test_explicit_throw& operator=(const test_explicit_throw&) throw(int) { throw 42; }
+test_explicit_throw& operator=(test_explicit_throw&&) throw(int) { throw 42; }
+~test_explicit_throw() throw(int) { throw 42; }
+};
+
+struct test_implicit_throw {
+test_implicit_throw() { throw 42; }
+test_implicit_throw(const test_implicit_throw&) { throw 42; }
+test_implicit_throw(test_implicit_throw&&) { throw 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'test_implicit_throw' which should not throw exceptions
+test_implicit_throw& operator=(const test_implicit_throw&) { throw 42; }
+test_implicit_throw& operator=(test_implicit_throw&&) { throw 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:26: warning: an exception may be thrown in function 'operator=' which should not throw exceptions
+~test_implicit_throw() { throw 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function '~test_implicit_throw' which should not throw exceptions
+};
+
+}}
Index: 

  1   2   >