[PATCH] D131879: [clang][analyzer] Errno modeling code refactor (NFC).

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd56a1c682477: [clang][analyzer] Errno modeling code refactor 
(NFC). (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131879

Files:
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -57,19 +57,6 @@
 using namespace clang;
 using namespace clang::ento;
 
-/// Produce a textual description of the state of \c errno (this describes the
-/// way how it is allowed to be used).
-/// The returned string is insertable into a longer warning message (in the form
-/// "the value 'errno' <...>").
-/// Currently only the \c errno_modeling::MustNotBeChecked state is supported.
-/// But later other kind of errno state may be needed if functions with special
-/// handling of \c errno are added.
-static const char *describeErrnoCheckState(errno_modeling::ErrnoCheckState CS) {
-  assert(CS == errno_modeling::MustNotBeChecked &&
- "Errno description not applicable.");
-  return "may be undefined after the call and should not be used";
-}
-
 namespace {
 class StdLibraryFunctionsChecker
 : public Checker {
@@ -392,45 +379,42 @@
   using ConstraintSet = std::vector;
 
   /// Define how a function affects the system variable 'errno'.
-  /// This works together with the ErrnoModeling and ErrnoChecker classes.
+  /// This works together with the \c ErrnoModeling and \c ErrnoChecker classes.
+  /// Currently 3 use cases exist: success, failure, irrelevant.
+  /// In the future the failure case can be customized to set \c errno to a
+  /// more specific constraint (for example > 0), or new case can be added
+  /// for functions which require check of \c errno in both success and failure
+  /// case.
   class ErrnoConstraintBase {
   public:
 /// Apply specific state changes related to the errno variable.
 virtual ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary,
   CheckerContext &C) const = 0;
-/// Get a description about what is applied to 'errno' and how is it allowed
-/// to be used. If ErrnoChecker generates a bug then this message is
-/// displayed as a note at the function call.
-/// It may return empty string if no note tag is to be added.
-virtual std::string describe(StringRef FunctionName) const { return ""; }
+/// Get a NoteTag about the changes made to 'errno' and the possible bug.
+/// It may return \c nullptr (if no bug report from \c ErrnoChecker is
+/// expected).
+virtual const NoteTag *describe(CheckerContext &C,
+StringRef FunctionName) const {
+  return nullptr;
+}
 
 virtual ~ErrnoConstraintBase() {}
 
   protected:
-/// Many of the descendant classes use this value.
-const errno_modeling::ErrnoCheckState CheckState;
-
-ErrnoConstraintBase(errno_modeling::ErrnoCheckState CS) : CheckState(CS) {}
+ErrnoConstraintBase() = default;
 
 /// This is used for conjure symbol for errno to differentiate from the
 /// original call expression (same expression is used for the errno symbol).
 static int Tag;
   };
 
-  /// Set value of 'errno' to be related to 0 in a specified way, with a
-  /// specified "errno check state". For example with \c BO_GT 'errno' is
-  /// constrained to be greater than 0. Use this for failure cases of functions.
-  class ZeroRelatedErrnoConstraint : public ErrnoConstraintBase {
-BinaryOperatorKind Op;
-
+  /// Set errno constraint at failure cases of standard functions.
+  /// Failure case: 'errno' becomes not equal to 0 and may or may not be checked
+  /// by the program. \c ErrnoChecker does not emit a bug report after such a
+  /// function call.
+  class FailureErrnoConstraint : public ErrnoConstraintBase {
   public:
-ZeroRelatedErrnoConstraint(clang::BinaryOperatorKind OpK,
-   errno_modeling::ErrnoCheckState CS)
-: ErrnoConstraintBase(CS), Op(OpK) {
-  assert(BinaryOperator::isComparisonOp(OpK));
-}
-
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
   const Summary &Summary,
   CheckerContext &C) const override {
@@ -440,62 +424,36 @@
C.getLocationContext(), C.getASTContext().IntTy,
C.blockCount())
   .castAs();
-  NonLoc ZeroVal =
-  SVB.makeZero

[clang] d56a1c6 - [clang][analyzer] Errno modeling code refactor (NFC).

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

Author: Balázs Kéri
Date: 2022-09-01T09:05:59+02:00
New Revision: d56a1c68247751e94c4fc46dda282643d3739689

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

LOG: [clang][analyzer] Errno modeling code refactor (NFC).

Some of the code used in StdLibraryFunctionsChecker is applicable to
other checkers, this is put into common functions. Errno related
parts of the checker are simplified and renamed. Documentations in
errno_modeling functions are updated.

This change makes it available to have more checkers that perform
modeling of some standard functions. These can set the errno state
with common functions and the bug report messages (note tags) can
look similar.

Reviewed By: steakhal, martong

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
index 618f7e97f6e89..a9e249de38047 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
@@ -264,6 +264,12 @@ bool isErrno(const Decl *D) {
   return false;
 }
 
+const char *describeErrnoCheckState(ErrnoCheckState CS) {
+  assert(CS == errno_modeling::MustNotBeChecked &&
+ "Errno description not applicable.");
+  return "may be undefined after the call and should not be used";
+}
+
 const NoteTag *getErrnoNoteTag(CheckerContext &C, const std::string &Message) {
   return C.getNoteTag([Message](PathSensitiveBugReport &BR) -> std::string {
 const MemRegion *ErrnoR = 
BR.getErrorNode()->getState()->get();
@@ -275,6 +281,32 @@ const NoteTag *getErrnoNoteTag(CheckerContext &C, const 
std::string &Message) {
   });
 }
 
+ProgramStateRef setErrnoForStdSuccess(ProgramStateRef State,
+  CheckerContext &C) {
+  return setErrnoState(State, MustNotBeChecked);
+}
+
+ProgramStateRef setErrnoForStdFailure(ProgramStateRef State, CheckerContext &C,
+  NonLoc ErrnoSym) {
+  SValBuilder &SVB = C.getSValBuilder();
+  NonLoc ZeroVal = SVB.makeZeroVal(C.getASTContext().IntTy).castAs();
+  DefinedOrUnknownSVal Cond =
+  SVB.evalBinOp(State, BO_NE, ErrnoSym, ZeroVal, SVB.getConditionType())
+  .castAs();
+  State = State->assume(Cond, true);
+  if (!State)
+return nullptr;
+  return setErrnoValue(State, C.getLocationContext(), ErrnoSym, Irrelevant);
+}
+
+const NoteTag *getNoteTagForStdSuccess(CheckerContext &C, llvm::StringRef Fn) {
+  return getErrnoNoteTag(
+  C, (Twine("Assuming that function '") + Twine(Fn) +
+  Twine("' is successful, in this case the value 'errno' ") +
+  Twine(describeErrnoCheckState(MustNotBeChecked)))
+ .str());
+}
+
 } // namespace errno_modeling
 } // namespace ento
 } // namespace clang

diff  --git a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h 
b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
index 3757e25e1afe6..d46a403d59a54 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
+++ b/clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
@@ -21,16 +21,23 @@ namespace clang {
 namespace ento {
 namespace errno_modeling {
 
+/// Describe how reads and writes of \c errno are handled by the checker.
 enum ErrnoCheckState : unsigned {
   /// We do not know anything about 'errno'.
+  /// Read and write is always allowed.
   Irrelevant = 0,
 
   /// Value of 'errno' should be checked to find out if a previous function 
call
   /// has failed.
+  /// When this state is set \c errno must be read by the program before a next
+  /// standard function call or other overwrite of \c errno follows, otherwise
+  /// a bug report is emitted.
   MustBeChecked = 1,
 
   /// Value of 'errno' is not allowed to be read, it can contain an unspecified
   /// value.
+  /// When this state is set \c errno is not allowed to be read by the program
+  /// until it is overwritten or invalidated.
   MustNotBeChecked = 2
 };
 
@@ -67,10 +74,38 @@ ProgramStateRef setErrnoState(ProgramStateRef State, 
ErrnoCheckState EState);
 /// declaration.
 bool isErrno(const Decl *D);
 
+/// Produce a textual description about how \c errno is allowed to be used
+/// (in a \c ErrnoCheckState).
+/// The returned string is insertable into a longer warning message in the form
+/// "the value 'errno' <...>".
+/// Currently only the \c errno_modeling::MustNotBeChecked state is supported,
+/// others are not used by the clients.
+const char *describeErrnoCheckState(ErrnoCheckState CS);
+
 /// Create a NoteTag that displays the message if t

[PATCH] D132831: [clang][Interp] Handle SubstNonTypeTemplateParmExprs

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

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



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:273
+const SubstNonTypeTemplateParmExpr *E) {
+  return this->visit(E->getReplacement());
+}

tbaeder wrote:
> tahonermann wrote:
> > erichkeane wrote:
> > > tbaeder wrote:
> > > > erichkeane wrote:
> > > > > Is there nothing special that has to happen when these are reference 
> > > > > parameters?  Can you at least add a test for that?
> > > > Umm, I don't really do references yet (I haven't tested them at all and 
> > > > I don't think they are implemented).
> > > Hmm... that is unfortunate.  We really should be doing references ASAP, 
> > > as they are going to be a pretty critical/oft-needed during testing kinda 
> > > thing.  I'd at least want a commented-out test that you can re-enable 
> > > ASAP.
> > Are not-yet-implemented features consistently implemented such that 
> > constant evaluation will fail with a (possibly unhelpful) diagnostic? As in 
> > https://godbolt.org/z/8vvdca9Ma? If so, can tests be added ahead of time 
> > with annotations for expected errors (essentially false positives to be 
> > addressed later)?
> Some are always (like unhandled AST node), some are not.
An example for this with a reference type is in https://reviews.llvm.org/D132997


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132831

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

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

during the original implementation of the fix-util I had the plan to provide an 
option for east-vs-west const, but during the discussions we came to the 
conclusion that `clang-format` should do this (the patches were already pending 
at that point in time).

if this option works, i think its ok to provide it. having this check on during 
development is maybe just annoying when the `const` always pops up at the 
"wrong" side.

but i think we should provide more test cases here, especially for the edge 
cases where `const` _must_ be on the right side, because it would otherwise 
apply to something else then intended. that was the original reason why i 
sticked to east-const, because thats always correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-alignment.cpp:43
+  // of the `*`.
+  int *IPtr = &I;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'IPtr' of type 'int *' 
can be declared 'const'

could you please provide more test cases with pointers in combination with

- macros
- typedefs
- template parameters
- `auto *`

I think we should be thorough here. The users might not have all details of 
where `const` applies in their forehead and big transformations of code-bases 
should definitely not break the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386

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

[PATCH] D125655: [HLSL] add -P option for dxc mode.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 457191.
python3kgae marked an inline comment as done.
python3kgae added a comment.

Switch to CLMode -P and -Fi.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125655

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/test/Driver/dxc_P.hlsl

Index: clang/test/Driver/dxc_P.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_P.hlsl
@@ -0,0 +1,16 @@
+// RUN: %clang_dxc -Tlib_6_7 -P -### %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7  -P -Fi"a.txt" -### %s 2>&1 | FileCheck %s --check-prefix=FI
+// RUN: %clang_dxc -Tlib_6_7  -Fo b.txt -P -Fi"a.txt" -### %s 2>&1 | FileCheck %s --check-prefix=WARNING
+
+// Make sure -P option flag which translated into "-E" + "-o" "a.txt".
+// CHECK:"-P"
+// CHECK-SAME: "-o" "dxc_P.i"
+
+// FI:"-P"
+// FI:"-Fia.txt"
+// FI-SAME:"-o" "a.txt"
+
+// Make sure got warning when -Fo and -P at same time.
+// WARNING: warning: output compiler options like -Fo ignored with Preprocess [-Woption-ignored]
+// WARNING:"-P"
+// WARNING-SAME:"-o" "a.txt"
Index: clang/lib/Driver/ToolChains/HLSL.cpp
===
--- clang/lib/Driver/ToolChains/HLSL.cpp
+++ clang/lib/Driver/ToolChains/HLSL.cpp
@@ -177,6 +177,9 @@
   }
 
   if (DAL->hasArg(options::OPT_o)) {
+if (DAL->hasArg(options::OPT__SLASH_P))
+  getDriver().Diag(diag::warn_drv_dxc_ignore_output_for_preprocess);
+
 // When run the whole pipeline.
 if (!DAL->hasArg(options::OPT_emit_llvm))
   // Emit obj if write to file.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3508,6 +3508,8 @@
  options::OPT_D,
  options::OPT_I,
  options::OPT_S,
+ options::OPT__SLASH_P,
+ options::OPT__SLASH_Fi,
  options::OPT_emit_llvm,
  options::OPT_emit_obj,
  options::OPT_disable_llvm_passes,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5455,7 +5455,8 @@
 
   llvm::PrettyStackTraceString CrashInfo("Computing output path");
   // Output to a user requested destination?
-  if (AtTopLevel && !isa(JA) && !isa(JA)) {
+  if (AtTopLevel && !isa(JA) && !isa(JA) &&
+  !(IsDXCMode() && C.getArgs().hasArg(options::OPT__SLASH_P))) {
 if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
   return C.addResultFile(FinalOutput->getValue(), &JA);
   }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6381,6 +6381,9 @@
 class CLFlag : Option<["/", "-"], name, KIND_FLAG>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
+class CLDXCFlag : Option<["/", "-"], name, KIND_FLAG>,
+  Group, Flags<[CLDXCOption, NoXarchOption]>;
+
 class CLCompileFlag : Option<["/", "-"], name, KIND_FLAG>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
@@ -6393,6 +6396,9 @@
 class CLCompileJoined : Option<["/", "-"], name, KIND_JOINED>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
+class CLDXCCompileJoined : Option<["/", "-"], name, KIND_JOINED>,
+  Group, Flags<[CLDXCOption, NoXarchOption]>;
+
 class CLIgnoredJoined : Option<["/", "-"], name, KIND_JOINED>,
   Group, Flags<[CLOption, NoXarchOption, HelpHidden]>;
 
@@ -6662,7 +6668,7 @@
   HelpText<"Set output executable file name">,
   MetaVarName<"">;
 def _SLASH_Fe_COLON : CLJoined<"Fe:">, Alias<_SLASH_Fe>;
-def _SLASH_Fi : CLCompileJoined<"Fi">,
+def _SLASH_Fi : CLDXCCompileJoined<"Fi">,
   HelpText<"Set preprocess output file name (with /P)">,
   MetaVarName<"">;
 def _SLASH_Fo : CLCompileJoined<"Fo">,
@@ -6697,7 +6703,7 @@
 def _SLASH_o : CLJoinedOrSeparate<"o">,
   HelpText<"Deprecated (set output file name); use /Fe or /Fe">,
   MetaVarName<"">;
-def _SLASH_P : CLFlag<"P">, HelpText<"Preprocess to file">;
+def _SLASH_P : CLDXCFlag<"P">, HelpText<"Preprocess to file">;
 def _SLASH_permissive : CLFlag<"permissive">,
   HelpText<"Enable some non conforming code to compile">;
 def _SLASH_permissive_ : CLFlag<"permissive-">,
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/cla

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

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

steakhal added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:799
+  /// we actually know their types.
+  QualType getApproximatedType() const {
+return sym->getType()->getPointeeType();

martong wrote:
> I think we should express that this returns the
> 
> 
>   - type of the pointee
>   - the type that is the "static" type, id est, the type of the declaration; 
> `void`, `char` and `base` in your example above.
> 
> In this sense, what about `getPointeeStaticType`?
> 
> 
I think it's important to discourage people from using this; hence the 
`approximated` calls attention that this might always be accurate.
I'm okay to add the `pointee` somewhere in the name.

I don't have a strong opinion about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132142

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

[PATCH] D132945: [clang] Skip re-building lambda expressions in parameters to consteval fns.

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added a comment.

Gentle ping for review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132945

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

[PATCH] D132997: [clang][Interp] Handle DeclRefExpr of reference types

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

tbaeder updated this revision to Diff 457193.

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

https://reviews.llvm.org/D132997

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/arrays.cpp
  clang/test/AST/Interp/references.cpp

Index: clang/test/AST/Interp/references.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/references.cpp
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+
+// ref-no-diagnostics
+
+constexpr int a = 10;
+constexpr const int &b = a;
+static_assert(a == b, "");
+
+constexpr int assignToReference() {
+  int a = 20;
+  int &b =a;
+
+  b = 100;
+  return a;
+}
+static_assert(assignToReference() == 100, "");
+
+
+constexpr void setValue(int &dest, int val) {
+  dest = val;
+}
+
+constexpr int checkSetValue() {
+  int l = 100;
+  setValue(l, 200);
+  return l;
+}
+static_assert(checkSetValue() == 200, "");
+
+constexpr int readLocalRef() {
+  int a = 20;
+  int &b = a;
+  return b;
+}
+static_assert(readLocalRef() == 20, "");
+
+constexpr int incRef() {
+  int a = 0;
+  int &b = a;
+
+  b = b + 1;
+
+  return a;
+}
+static_assert(incRef() == 1, "");
+
+
+template
+constexpr void Plus3(int &A) {
+  A = V + 3;
+}
+constexpr int foo = 4;
+
+constexpr int callTemplate() {
+  int a = 3;
+  Plus3(a);
+  return a;
+}
+static_assert(callTemplate() == 7, "");
+
+
+constexpr int& getValue(int *array, int index) {
+  return array[index];
+}
+constexpr int testGetValue() {
+  int values[] = {1, 2, 3, 4};
+  getValue(values, 2) = 30;
+  return values[2];
+}
+static_assert(testGetValue() == 30, "");
+
+// FIXME: ExprWithCleanups + MaterializeTemporaryExpr not implemented
+constexpr const int &MCE = 1; // expected-error{{must be initialized by a constant expression}}
Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -45,6 +45,13 @@
 static_assert(getElementOf(foo[0], 1) == &m, "");
 
 
+template 
+constexpr T& getElementOfArray(T (&array)[N], int I) {
+  return array[I];
+}
+static_assert(getElementOfArray(foo[2], 3) == &m, "");
+
+
 constexpr int data[] = {5, 4, 3, 2, 1};
 static_assert(data[0] == 4, ""); // expected-error{{failed}} \
  // expected-note{{5 == 4}} \
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -845,15 +845,38 @@
 template 
 bool ByteCodeExprGen::VisitDeclRefExpr(const DeclRefExpr *E) {
   const auto *Decl = E->getDecl();
+  bool IsReference = Decl->getType()->isReferenceType();
+  bool FoundDecl = false;
 
   if (auto It = Locals.find(Decl); It != Locals.end()) {
 const unsigned Offset = It->second.Offset;
-return this->emitGetPtrLocal(Offset, E);
+if (!this->emitGetPtrLocal(Offset, E))
+  return false;
+
+FoundDecl = true;
   } else if (auto GlobalIndex = P.getGlobal(Decl)) {
-return this->emitGetPtrGlobal(*GlobalIndex, E);
+if (!this->emitGetPtrGlobal(*GlobalIndex, E))
+  return false;
+
+FoundDecl = true;
   } else if (const auto *PVD = dyn_cast(Decl)) {
-if (auto It = this->Params.find(PVD); It != this->Params.end())
-  return this->emitGetPtrParam(It->second, E);
+if (auto It = this->Params.find(PVD); It != this->Params.end()) {
+  if (!this->emitGetPtrParam(It->second, E))
+return false;
+
+  FoundDecl = true;
+}
+  }
+
+  // References are implemented using pointers, so when we get here,
+  // we have a pointer to a pointer, which we need to de-reference once.
+  if (FoundDecl) {
+if (IsReference) {
+  if (!this->emitLoadPop(PT_Ptr, E))
+return false;
+}
+
+return true;
   }
 
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D132997: [clang][Interp] Handle DeclRefExpr of reference types

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

tbaeder added a comment.

In D132997#3763094 , @shafik wrote:

> I think like @aaron.ballman was saying in another PR you should aim for as 
> complete set of tests as possible even if you have to comment one that can't 
> work yet. For example cases that would involve `MemberExpr` or `UsingShadow` 
> for example as well as the cases you mentioned not implemented in your 
> description.

I get it for the `MaterializeTemporaryExpr`s, but I don't understand why I 
would add tests using `MemberExpr` now and not when I work on record types (and 
references have already landed).


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

https://reviews.llvm.org/D132997

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

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 457199.
njames93 marked 3 inline comments as done.
njames93 added a comment.

Fix documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-braces.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-nested.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-split.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
@@ -0,0 +1,144 @@
+// RUN: %check_clang_tidy %s readability-use-early-exits %t -- \
+// RUN:   -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 1}}"
+
+// Run the check with the braces around statements check also enabled, but
+// ShortStatementLines is set so that we shouldn't add braces.
+// RUN: %check_clang_tidy %s readability-use-early-exits,readability-braces-around-statements -format-style=llvm %t -- \
+// RUN:   -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 1, \
+// RUN:readability-braces-around-statements.ShortStatementLines: 2}}"
+
+// Just to hit the threshold
+void padLines();
+
+void nomralFunc(int *A) {
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+  if (A) {
+*A = 10;
+  }
+  //   CHECK-FIXES: if (!A)
+  //  CHECK-FIXES-NEXT: return;
+  //  CHECK-FIXES-NEXT: *A = 10;
+}
+
+void funcWithTrailing(int *B) {
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+  if (B) {
+*B = 10;
+  }
+  return;
+  ;
+  //   CHECK-FIXES: if (!B)
+  //  CHECK-FIXES-NEXT: return;
+  //  CHECK-FIXES-NEXT: *B = 10;
+}
+
+void normal(int A, int B) {
+
+  while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+if (A == B) {
+  padLines();
+}
+  }
+  //   CHECK-FIXES:  while (true) {
+  //  CHECK-FIXES-NEXT: if (A != B)
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT: padLines();
+
+  // Eat continue and empty nulls
+  while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+if (A != B) {
+  padLines();
+}
+continue;
+;
+continue;
+  }
+  //   CHECK-FIXES:  while (true) {
+  //  CHECK-FIXES-NEXT: if (A == B)
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT: padLines();
+}
+
+void toShort(int A, int B) {
+  while (true) {
+if (A == B) 
+{ }
+  }
+}
+
+void hasElse(int A, int B) {
+  while (true) {
+if (A == B) {
+
+} else {
+}
+  }
+}
+
+void hasTrailingStmt(int A, int B) {
+  while (true) {
+if (A == B) {
+}
+padLines();
+  }
+}
+
+void nested(int A, int B) {
+  // if (A > B) {
+  // CHECK-MESSAGES: [[@LINE+6]]:5: warning: use early exit
+  // if (B < A) {
+  // CHECK-MESSAGES: [[@LINE+5]]:7: warning: use early exit
+  // if (A == B) {
+  // CHECK-MESSAGES: [[@LINE+4]]:9: warning: use early exit
+  while (true) { // Nested
+if (A > B) {
+  if (B < A) {
+if (A != B) {
+  padLines();
+}
+  }
+}
+  } // EndLoop
+  //   CHECK-FIXES: while (true) { // Nested
+  //  CHECK-FIXES-NEXT: if (A <= B)
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT: if (B >= A)
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT: if (A == B)
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT: padLines();
+}
+
+void badNested(bool B) {
+  // Ensure check doesn't check for nested `if` if outer `if` has else.
+  while (true) {
+if (B) {
+  if (B) {
+  }
+} else {
+}
+  }
+}
+
+void semiNested(int A, int B) {
+  // CHECK-MESSAGES: [[@LINE+2]]:5: warning: use early exit
+  while (true) { // SemiNested
+if (A > B) {
+  if (B < A) {
+if (A != B) {
+  padLines();
+}
+  } else {
+  }
+}
+  }
+  //   CHECK-FIXES: while (true) { // SemiNested
+  //  CHECK-FIXES-NEXT:   if (A <= B)
+  //  CHECK-FIXES-NEXT: continue;
+  //  CHECK-FIXES-NEXT:   if (B < A) {
+  //  CHECK-FIXES-NEXT: if (A != B) {
+  //  CHECK-FIXES-NEXT:   padLines();
+  //  CHECK-FIXES-NEXT: }
+  //  CHECK-FIXES-NEXT: } else {
+  //  CHECK-FIXES-NEXT: }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst:63
+void Process(bool A, bool B) {
+  if (A && B) {
+// Long processing.

JonasToth wrote:
> njames93 wrote:
> > JonasToth wrote:
> > > if this option is false, the transformation would be `if(!(A && B))`, 
> > > right?
> > > 
> > > should demorgan rules be applied or at least be mentioned here? I think 
> > > transforming to `if (!A || !B)` is at least a viable option for enough 
> > > users.
> > Once this is in, I plan to merge some common code with the 
> > simplify-boolean-expr logic for things like demorgan processing. Right now 
> > the transformation happens, the simplify boolean suggests a demorgan 
> > transformation of you run the output through clang tidy.
> a short reference to the `readability-simplify-boolean-expr` check in the 
> user facing docs would be great.
> i personally find it ok if the users "pipe" multiple checks together to reach 
> a final transformation.
> 
> would this check then use the same settings as the 
> `readability-simplify-boolean-expr` check? (that of course off topic and does 
> not relate to this patch :) )
I'm not sure it's really necessary to mention that the fix would likely need 
another fix, besides that comment would just be removed in the follow up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

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

[clang] 7e19d53 - [NFC] Emit builtin coroutine calls uniforally

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-09-01T16:31:51+08:00
New Revision: 7e19d53da44942de9b373028828c276648a8f2a4

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

LOG: [NFC] Emit builtin coroutine calls uniforally

All the coroutine builtins were emitted in EmitCoroutineIntrinsic except
__builtin_coro_size. This patch tries to emit all the corotine builtins
uniformally.

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGCoroutine.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f2d0b20c00c4..cc3cf9ab46b5 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -4647,14 +4647,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
   case Builtin::BI__fastfail:
 return RValue::get(EmitMSVCBuiltinExpr(MSVCIntrin::__fastfail, E));
 
-  case Builtin::BI__builtin_coro_size: {
-auto & Context = getContext();
-auto SizeTy = Context.getSizeType();
-auto T = Builder.getIntNTy(Context.getTypeSize(SizeTy));
-Function *F = CGM.getIntrinsic(Intrinsic::coro_size, T);
-return RValue::get(Builder.CreateCall(F));
-  }
-
   case Builtin::BI__builtin_coro_id:
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_id);
   case Builtin::BI__builtin_coro_promise:
@@ -4679,6 +4671,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_end);
   case Builtin::BI__builtin_coro_suspend:
 return EmitCoroutineIntrinsic(E, Intrinsic::coro_suspend);
+  case Builtin::BI__builtin_coro_size:
+return EmitCoroutineIntrinsic(E, Intrinsic::coro_size);
 
   // OpenCL v2.0 s6.13.16.2, Built-in pipe read and write functions
   case Builtin::BIread_pipe:

diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 177d85ff699c..c0fcd632ea0d 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -676,6 +676,13 @@ RValue CodeGenFunction::EmitCoroutineIntrinsic(const 
CallExpr *E,
 auto NullPtr = llvm::ConstantPointerNull::get(Builder.getInt8PtrTy());
 return RValue::get(NullPtr);
   }
+  case llvm::Intrinsic::coro_size: {
+auto &Context = getContext();
+auto SizeTy = Context.getSizeType();
+auto T = Builder.getIntNTy(Context.getTypeSize(SizeTy));
+llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::coro_size, T);
+return RValue::get(Builder.CreateCall(F));
+  }
   // The following three intrinsics take a token parameter referring to a token
   // returned by earlier call to @llvm.coro.id. Since we cannot represent it in
   // builtins, we patch it up here.



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

[PATCH] D132945: [clang] Skip re-building lambda expressions in parameters to consteval fns.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

Thanks, that's a nice find!

Could you also add a test that mentions uses `consteval` functions in lambda 
captures inside immediate invocations?
Something like `consteval_foo([x = consteval_foo(1)]() consteval { return x; 
})` and maybe also without `consteval` on lambda parameter list.
I suspect this should work as expected, just wanted to make sure we capture 
this in tests




Comment at: clang/docs/ReleaseNotes.rst:196
+
+- Skip re-building lambda expressions when they appear as parameters to an 
immediate invocation.
+  This fixes `GH56183 `_,

NIT



Comment at: clang/lib/Sema/SemaExpr.cpp:17601-17605
+ExprResult TransformLambdaExpr(LambdaExpr *E) {
+  // Lambdas must be built already. They must not be re-built as it always
+  // creates new types.
+  return E;
+}

NIT: I tried to expand the meaning of term 'built' to make the comment a bit 
clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132945

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

[PATCH] D132473: [Docs][OpenCL][SPIR-V] Release 15 notes for Clang

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in 
https://github.com/llvm/llvm-project/commit/5e1b36ced538cfc80f2400b27e346d2175b04092.


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

https://reviews.llvm.org/D132473

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

[clang] c640b8c - [NFC][clang] LLVM_FALLTHROUGH => [[fallthrough]]

[via cfe-commits]

Author: Sheng
Date: 2022-09-01T09:19:00Z
New Revision: c640b8ce33f7d128c4d25c282bd245ae7177cf04

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

LOG: [NFC][clang] LLVM_FALLTHROUGH => [[fallthrough]]

Added: 


Modified: 
clang/lib/Parse/ParseExprCXX.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index b2e9aa1f6a14..207977f7176d 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -3065,7 +3065,7 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec &SS, 
ParsedType ObjectType,
   << Tok.getIdentifierInfo()->getName() << 0;
   goto ParseIdentifier;
 }
-LLVM_FALLTHROUGH;
+[[fallthrough]];
   default:
 Diag(Tok, diag::err_expected_unqualified_id) << getLangOpts().CPlusPlus;
 return true;



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

[PATCH] D130768: [OpenCL][SPIR-V] Add test for extern functions with a pointer

[Anastasia Stulova via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6b1a04529c8f: [OpenCL][SPIR-V] Test extern functions with a 
pointer arg. (authored by Anastasia).
Herald added a subscriber: ldrumm.
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D130768?vs=448602&id=457208#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130768

Files:
  clang/test/CodeGenOpenCL/opaque-ptr-spirv.cl


Index: clang/test/CodeGenOpenCL/opaque-ptr-spirv.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/opaque-ptr-spirv.cl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -o - -triple spirv64 %s | 
FileCheck %s
+
+// Check that we have a way to recover pointer
+// types for extern function prototypes (see PR56660).
+extern void foo(global int * ptr);
+kernel void k(global int * ptr) {
+  foo(ptr);
+}
+//CHECK: define spir_kernel void @k(i32 {{.*}}*
+//CHECK: declare spir_func void @foo(i32 {{.*}}*


Index: clang/test/CodeGenOpenCL/opaque-ptr-spirv.cl
===
--- /dev/null
+++ clang/test/CodeGenOpenCL/opaque-ptr-spirv.cl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -o - -triple spirv64 %s | FileCheck %s
+
+// Check that we have a way to recover pointer
+// types for extern function prototypes (see PR56660).
+extern void foo(global int * ptr);
+kernel void k(global int * ptr) {
+  foo(ptr);
+}
+//CHECK: define spir_kernel void @k(i32 {{.*}}*
+//CHECK: declare spir_func void @foo(i32 {{.*}}*
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 6b1a045 - [OpenCL][SPIR-V] Test extern functions with a pointer arg.

[Anastasia Stulova via cfe-commits]

Author: Anastasia Stulova
Date: 2022-09-01T10:22:47+01:00
New Revision: 6b1a04529c8fba4019b3a7f56fe6e4938f3a188a

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

LOG: [OpenCL][SPIR-V] Test extern functions with a pointer arg.

Added a test case that enhances coverage of opaque pointers
particularly for the problematic case with extern functions
for which there is no solution found for type recovery.

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

Added: 
clang/test/CodeGenOpenCL/opaque-ptr-spirv.cl

Modified: 


Removed: 




diff  --git a/clang/test/CodeGenOpenCL/opaque-ptr-spirv.cl 
b/clang/test/CodeGenOpenCL/opaque-ptr-spirv.cl
new file mode 100644
index ..93220ef98f18
--- /dev/null
+++ b/clang/test/CodeGenOpenCL/opaque-ptr-spirv.cl
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -o - -triple spirv64 %s | 
FileCheck %s
+
+// Check that we have a way to recover pointer
+// types for extern function prototypes (see PR56660).
+extern void foo(global int * ptr);
+kernel void k(global int * ptr) {
+  foo(ptr);
+}
+//CHECK: define spir_kernel void @k(i32 {{.*}}*
+//CHECK: declare spir_func void @foo(i32 {{.*}}*



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

[PATCH] D131217: [clang][WebAssembly] Pass `-Wl,-no-type-check` through to the MC layer

[Sam Clegg via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG849df8f6f0a6: [clang][WebAssembly] Pass 
`-Wa,--no-type-check` through to the MC layer (authored by sbc100).

Changed prior to commit:
  https://reviews.llvm.org/D131217?vs=454128&id=457215#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131217

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/notypecheck.s
  clang/tools/driver/cc1as_main.cpp


Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -134,6 +134,7 @@
   unsigned NoExecStack : 1;
   unsigned FatalWarnings : 1;
   unsigned NoWarn : 1;
+  unsigned NoTypeCheck : 1;
   unsigned IncrementalLinkerCompatible : 1;
   unsigned EmbedBitcode : 1;
 
@@ -166,6 +167,7 @@
 NoExecStack = 0;
 FatalWarnings = 0;
 NoWarn = 0;
+NoTypeCheck = 0;
 IncrementalLinkerCompatible = 0;
 Dwarf64 = 0;
 DwarfVersion = 0;
@@ -304,6 +306,7 @@
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
   Opts.NoWarn = Args.hasArg(OPT_massembler_no_warn);
+  Opts.NoTypeCheck = Args.hasArg(OPT_mno_type_check);
   Opts.RelocationModel =
   std::string(Args.getLastArgValue(OPT_mrelocation_model, "pic"));
   Opts.TargetABI = std::string(Args.getLastArgValue(OPT_target_abi));
@@ -468,6 +471,7 @@
 
   MCOptions.MCNoWarn = Opts.NoWarn;
   MCOptions.MCFatalWarnings = Opts.FatalWarnings;
+  MCOptions.MCNoTypeCheck = Opts.NoTypeCheck;
   MCOptions.ABIName = Opts.TargetABI;
 
   // FIXME: There is a bit of code duplication with addPassesToEmitFile.
Index: clang/test/Driver/notypecheck.s
===
--- /dev/null
+++ clang/test/Driver/notypecheck.s
@@ -0,0 +1,13 @@
+# REQUIRES: webassembly-registered-target
+
+# RUN: %clang -### %s -c -o tmp.o -target wasm32-unknown-unknown 
-Wa,--no-type-check 2>&1 | FileCheck %s
+# CHECK: "-cc1as" {{.*}} "-mno-type-check"
+
+# Verify that without -Wa,--no-type-check the assembler will error out
+# RUN: not %clang %s -c -o tmp.o -target wasm32-unknown-unknown 2>&1 | 
FileCheck --check-prefix=ERROR %s
+# ERROR: error: popped i64, expected i32
+
+foo:
+  .functype  foo () -> (i32)
+  i64.const 42
+  end_function
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2580,6 +2580,13 @@
   switch (C.getDefaultToolChain().getArch()) {
   default:
 break;
+  case llvm::Triple::wasm32:
+  case llvm::Triple::wasm64:
+if (Value == "--no-type-check") {
+  CmdArgs.push_back("-mno-type-check");
+  continue;
+}
+break;
   case llvm::Triple::thumb:
   case llvm::Triple::thumbeb:
   case llvm::Triple::arm:
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5286,6 +5286,9 @@
"Note this may change .s semantics and shouldn't generally be used "
"on compiler-generated code.">,
   MarshallingInfoFlag>;
+def mno_type_check : Flag<["-"], "mno-type-check">,
+  HelpText<"Don't perform type checking of the assembly code (wasm only)">,
+  MarshallingInfoFlag>;
 def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
   HelpText<"Disable implicit builtin knowledge of math functions">,
   MarshallingInfoFlag>;
Index: clang/include/clang/Basic/CodeGenOptions.def
===
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -180,6 +180,7 @@
 CODEGENOPT(FatalWarnings , 1, 0) ///< Set when -Wa,--fatal-warnings is
  ///< enabled.
 CODEGENOPT(NoWarn, 1, 0) ///< Set when -Wa,--no-warn is enabled.
+CODEGENOPT(NoTypeCheck   , 1, 0) ///< Set when -Wa,--no-type-check is 
enabled.
 CODEGENOPT(MisExpect , 1, 0) ///< Set when -Wmisexpect is enabled
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is 
enabled.
 CODEGENOPT(NoInlineLineTables, 1, 0) ///< Whether debug info should contain


Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -134,6 +134,7 @@
   unsigned NoExecStack : 1;
   unsigned FatalWarnings : 1;
   unsigned NoWarn : 1;
+  unsigned NoTypeCheck : 1;
   unsigned IncrementalLinkerCompatible : 1;
   unsigned Emb

[clang] 849df8f - [clang][WebAssembly] Pass `-Wa,--no-type-check` through to the MC layer

[Sam Clegg via cfe-commits]

Author: Sam Clegg
Date: 2022-09-01T02:56:58-07:00
New Revision: 849df8f6f0a6ded83e36aa52b6daa6b950289804

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

LOG: [clang][WebAssembly] Pass `-Wa,--no-type-check` through to the MC layer

I took as an example the `-Wa,--noexecstack` clang flag that maps down
to `cc1 -mnoexecstack`.

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

Added: 
clang/test/Driver/notypecheck.s

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/tools/driver/cc1as_main.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 87c75bb98d433..55c6940fb34c1 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -180,6 +180,7 @@ CODEGENOPT(NoExecStack   , 1, 0) ///< Set when 
-Wa,--noexecstack is enabled.
 CODEGENOPT(FatalWarnings , 1, 0) ///< Set when -Wa,--fatal-warnings is
  ///< enabled.
 CODEGENOPT(NoWarn, 1, 0) ///< Set when -Wa,--no-warn is enabled.
+CODEGENOPT(NoTypeCheck   , 1, 0) ///< Set when -Wa,--no-type-check is 
enabled.
 CODEGENOPT(MisExpect , 1, 0) ///< Set when -Wmisexpect is enabled
 CODEGENOPT(EnableSegmentedStacks , 1, 0) ///< Set when -fsplit-stack is 
enabled.
 CODEGENOPT(NoInlineLineTables, 1, 0) ///< Whether debug info should contain

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 2aef6d5d95720..ee1012efc086d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5286,6 +5286,9 @@ def msave_temp_labels : Flag<["-"], "msave-temp-labels">,
"Note this may change .s semantics and shouldn't generally be used "
"on compiler-generated code.">,
   MarshallingInfoFlag>;
+def mno_type_check : Flag<["-"], "mno-type-check">,
+  HelpText<"Don't perform type checking of the assembly code (wasm only)">,
+  MarshallingInfoFlag>;
 def fno_math_builtin : Flag<["-"], "fno-math-builtin">,
   HelpText<"Disable implicit builtin knowledge of math functions">,
   MarshallingInfoFlag>;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 1fa83e7121c95..eb7ad39d7edc7 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2580,6 +2580,13 @@ static void 
CollectArgsForIntegratedAssembler(Compilation &C,
   switch (C.getDefaultToolChain().getArch()) {
   default:
 break;
+  case llvm::Triple::wasm32:
+  case llvm::Triple::wasm64:
+if (Value == "--no-type-check") {
+  CmdArgs.push_back("-mno-type-check");
+  continue;
+}
+break;
   case llvm::Triple::thumb:
   case llvm::Triple::thumbeb:
   case llvm::Triple::arm:

diff  --git a/clang/test/Driver/notypecheck.s b/clang/test/Driver/notypecheck.s
new file mode 100644
index 0..f6e78d6791182
--- /dev/null
+++ b/clang/test/Driver/notypecheck.s
@@ -0,0 +1,13 @@
+# REQUIRES: webassembly-registered-target
+
+# RUN: %clang -### %s -c -o tmp.o -target wasm32-unknown-unknown 
-Wa,--no-type-check 2>&1 | FileCheck %s
+# CHECK: "-cc1as" {{.*}} "-mno-type-check"
+
+# Verify that without -Wa,--no-type-check the assembler will error out
+# RUN: not %clang %s -c -o tmp.o -target wasm32-unknown-unknown 2>&1 | 
FileCheck --check-prefix=ERROR %s
+# ERROR: error: popped i64, expected i32
+
+foo:
+  .functype  foo () -> (i32)
+  i64.const 42
+  end_function

diff  --git a/clang/tools/driver/cc1as_main.cpp 
b/clang/tools/driver/cc1as_main.cpp
index 5498810d835cd..5e528a00aa778 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -134,6 +134,7 @@ struct AssemblerInvocation {
   unsigned NoExecStack : 1;
   unsigned FatalWarnings : 1;
   unsigned NoWarn : 1;
+  unsigned NoTypeCheck : 1;
   unsigned IncrementalLinkerCompatible : 1;
   unsigned EmbedBitcode : 1;
 
@@ -166,6 +167,7 @@ struct AssemblerInvocation {
 NoExecStack = 0;
 FatalWarnings = 0;
 NoWarn = 0;
+NoTypeCheck = 0;
 IncrementalLinkerCompatible = 0;
 Dwarf64 = 0;
 DwarfVersion = 0;
@@ -304,6 +306,7 @@ bool 
AssemblerInvocation::CreateFromArgs(AssemblerInvocation &Opts,
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
   Opts.NoWarn = Args.hasArg(OPT_massembler_no_warn);
+  Opts.NoTypeCheck = Args.hasArg(OPT_mno_type_check);
   Opts.RelocationModel =
   std::string(Args.getLastArgValue(OPT_mrelocation_model, "pic"));
   Opts.TargetABI = std::string(

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

[Wei Yi Tee via cfe-commits]

Author: Wei Yi Tee
Date: 2022-09-01T10:15:53Z
New Revision: d931ac9e27cab964c65078c80e4488185c62b3d8

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

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

`MatchSwitch` currently takes in matchers and functions for the `Stmt` class.

This patch generalises the match switch utility (renamed to `ASTMatchSwitch`) 
to work for different AST node types by introducing a template argument which 
is the base type for the AST nodes that the match switch will handle.

A `CFGMatchSwitch` is introduced as a wrapper around multiple `ASTMatchSwitch`s 
for different base types. It works by unwrapping `CFGElement`s into their 
contained AST nodes and passing the nodes to the relevant `ASTMatchSwitch`. The 
`CFGMatchSwitch` currently only handles `CFGStmt` and `CFGInitializer`.

Reviewed By: gribozavr2, sgatev

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

Added: 
clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp

Modified: 
clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h 
b/clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
new file mode 100644
index 0..ecd8558970f93
--- /dev/null
+++ b/clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
@@ -0,0 +1,98 @@
+//=== CFGMatchSwitch.h --*- 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
+//
+//===--===//
+//
+//  This file defines the `CFGMatchSwitch` abstraction for building a "switch"
+//  statement for control flow graph elements. Each case of the switch is
+//  defined by an ASTMatcher which is applied on the AST node contained in the
+//  input `CFGElement`.
+//
+//  Currently, the `CFGMatchSwitch` only handles `CFGElement`s of
+//  `Kind::Statement` and `Kind::Initializer`.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_CFGMATCHSWITCH_H_
+#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_CFGMATCHSWITCH_H_
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Stmt.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
+#include 
+#include 
+
+namespace clang {
+namespace dataflow {
+
+template 
+using CFGMatchSwitch =
+std::function;
+
+/// Collects cases of a "match switch": a collection of matchers paired with
+/// callbacks, which together define a switch that can be applied to an AST 
node
+/// contained in a CFG element.
+template  class CFGMatchSwitchBuilder {
+public:
+  /// Registers an action `A` for `CFGStmt`s that will be triggered by the 
match
+  /// of the pattern `M` against the `Stmt` contained in the input `CFGStmt`.
+  ///
+  /// Requirements:
+  ///
+  ///  `NodeT` should be derived from `Stmt`.
+  template 
+  CFGMatchSwitchBuilder &&
+  CaseOfCFGStmt(MatchSwitchMatcher M,
+MatchSwitchAction A) && {
+std::move(StmtBuilder).template CaseOf(M, A);
+return std::move(*this);
+  }
+
+  /// Registers an action `A` for `CFGInitializer`s that will be triggered by
+  /// the match of the pattern `M` against the `CXXCtorInitializer` contained 
in
+  /// the input `CFGInitializer`.
+  ///
+  /// Requirements:
+  ///
+  ///  `NodeT` should be derived from `CXXCtorInitializer`.
+  template 
+  CFGMatchSwitchBuilder &&
+  CaseOfCFGInit(MatchSwitchMatcher M,
+MatchSwitchAction A) && {
+std::move(InitBuilder).template CaseOf(M, A);
+return std::move(*this);
+  }
+
+  CFGMatchSwitch Build() && {
+return [StmtMS = std::move(StmtBuilder).Build(),
+InitMS = std::move(InitBuilder).Build()](const CFGElement &Element,
+ ASTContext &Context,
+ State &S) -> Result {
+  switch (Element.getKind()) {
+  case CFGElement::Initializer:
+return InitMS(*Element.castAs().getInitializer(),
+  Context, S);
+  case CFGElement::Statement:
+  case CFGElement::Constructor:
+  case CFGElement::CXXRecordTypedCall:
+return StmtMS(*Element.castAs().getStmt(), Context, S);
+

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

[weiyi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd931ac9e27ca: [clang][dataflow] Generalise match switch 
utility to other AST types and add a… (authored by wyt).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131616

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

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

[PATCH] D132855: [OpenMP] Extend the lit test for uses_allocators in target region

[Animesh Kumar via Phabricator via cfe-commits]

animeshk-amd updated this revision to Diff 457219.
animeshk-amd added a comment.

Extended the LIT test for `defaultmap` clause in the target region
with respect to the default OpenMP version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132855

Files:
  clang/test/OpenMP/target_map_codegen_10.cpp
  clang/test/OpenMP/target_uses_allocators.c

Index: clang/test/OpenMP/target_uses_allocators.c
===
--- clang/test/OpenMP/target_uses_allocators.c
+++ clang/test/OpenMP/target_uses_allocators.c
@@ -1,7 +1,7 @@
 // Test host codegen.
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50  -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50  -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 
 // expected-no-diagnostics
 #ifndef HEADER
@@ -42,3 +42,59 @@
 }
 
 #endif
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, ptr null)
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr null)
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, ptr inttoptr (i64 1 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr inttoptr (i64 1 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, ptr inttoptr (i64 2 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr inttoptr (i64 2 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, ptr inttoptr (i64 3 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr inttoptr (i64 3 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, ptr inttoptr (i64 4 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr inttoptr (i64 4 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, ptr inttoptr (i64 5 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr inttoptr (i64 5 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, ptr inttoptr (i64 6 to ptr))
+// CHECK-NEXT: %[[#R1:]] = load i32, ptr %x.addr, align 4
+// CHECK-NEXT: store i32 %[[#R1]], ptr %.x..void.addr, align 4
+// CHECK-NEXT: call void @__kmpc_free(i32 %[[#R0]], ptr %.x..void.addr, ptr inttoptr (i64 6 to ptr))
+
+// CHECK: %[[#R0:]] = call i32 @__kmpc_global_thread_num(ptr @1)
+// CHECK-NEXT: store i64 %x, ptr %x.addr, align 8
+// CHECK-NEXT: %.x..void.addr = call ptr @__kmpc_alloc(i32 %[[#R0]], i64 4, ptr inttoptr (i64 7 to ptr))
+// CHECK-NEXT: %[[#R1:]] = lo

[PATCH] D133085: [clang] trim trailing space in format tests. NFC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133085#3763233 , @inclyc wrote:

> In D133085#3763198 , @ChuanqiXu 
> wrote:
>
>> Do you have commit access? If you have, I remember LLVM encourages to land 
>> such fixes directly without reviewed. (+ @aaron.ballman to make sure)
>
> Thanks! I'm just not sure whether these changes are necessary or not. :)

It's always okay to do an NFC commit which strips trailing whitespace, fixes a 
typo or misspelling, adds/removes newlines, or that sort of change in a 
targeted way. No need to review it unless you want a second set of eyes on the 
changes for some reason. We typically don't want a whole-project NFC change to 
do that sort of thing, though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133085

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

[clang-tools-extra] cc09b81 - [CTE][docs] Fix bad links in ReleaseNotes

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2022-09-01T12:10:53+01:00
New Revision: cc09b81265e9543b9b78a2687e7b5c818a137a59

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

LOG: [CTE][docs] Fix bad links in ReleaseNotes

Added: 


Modified: 
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 9bd3a5fc799cf..55c8c341bff3a 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,7 +130,8 @@ Changes in existing checks
   would be emitted for uninitialized members of an anonymous union despite
   there being an initializer for one of the other members.
 
-- Improved `modernize-use-emplace 
`_ check.
+- Improved :doc:`modernize-use-emplace 
`
+  check.
 
   The check now supports detecting inefficient invocations of ``push`` and
   ``push_front`` on STL-style containers and replacing them with ``emplace``
@@ -140,7 +141,8 @@ Changes in existing checks
   ``push_front`` on STL-style containers and replacing them with 
``emplace_back``,
   ``emplace`` or ``emplace_front``.
 
-- Improved `modernize-use-equals-default 
`_ check.
+- Improved :doc:`modernize-use-equals-default 
`
+  check.
 
   The check now skips unions since in this case a default constructor with 
empty body
   is not equivalent to the explicitly defaulted one.



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

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

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:799
+  /// we actually know their types.
+  QualType getApproximatedType() const {
+return sym->getType()->getPointeeType();

steakhal wrote:
> martong wrote:
> > I think we should express that this returns the
> > 
> > 
> >   - type of the pointee
> >   - the type that is the "static" type, id est, the type of the 
> > declaration; `void`, `char` and `base` in your example above.
> > 
> > In this sense, what about `getPointeeStaticType`?
> > 
> > 
> I think it's important to discourage people from using this; hence the 
> `approximated` calls attention that this might always be accurate.
> I'm okay to add the `pointee` somewhere in the name.
> 
> I don't have a strong opinion about this.
Isn't precise enough to say that this returns the `static` type? I don't quite 
get the exact definition of the "approximation" we do here, so I guess others 
will not get that either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132142

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

[PATCH] D133102: [clang-tidy] Extend simplify-boolean-expr check

[Nathan James via Phabricator via cfe-commits]

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

Extend the check to catch these conditions:

  if (e) return true; else return x;  // return e || x;
  if (e) return false; else return x; // return !e && x;
  if (e) return true; return x;   // return e || x;
  if (e) return false; return x;  // return !e && x;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133102

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-case.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr.cpp
@@ -357,10 +357,6 @@
   if (i == j) return (i == 0); else return false;
 }
 
-bool conditional_return_statements_else_expr(int i, int j) {
-  if (i == j) return true; else return (i == 0);
-}
-
 bool negated_conditional_return_statements(int i) {
   if (i == 0) return false; else return true;
 }
@@ -387,6 +383,46 @@
 // CHECK-FIXES-NEXT: {{^}}  return i == 1;{{$}}
 // CHECK-FIXES-NEXT: {{^}$}}
 
+bool getFallback();
+
+bool conditional_return_complex(bool B) {
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: return B ||  getFallback();
+  if (B)
+return true;
+  else
+return getFallback();
+}
+
+bool conditional_return_complex_compound(bool B) {
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: return B ||  getFallback();
+  if (B) {
+return true;
+  } else {
+return getFallback();
+  }
+}
+
+bool conditional_return_complex_compound_negated(bool B) {
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: return !B &&  getFallback();
+  if (B) {
+return false;
+  } else {
+return getFallback();
+  }
+}
+
+bool conditional_return_complex_negated(bool B) {
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: return !B &&  getFallback();
+  if (B)
+return false;
+  else
+return getFallback();
+}
+
 bool negated_conditional_compound_return_statements(int i) {
   if (i == 1) {
 return false;
@@ -745,6 +781,26 @@
 // CHECK-FIXES: {{^  return i <= 10;$}}
 // CHECK-FIXES: {{^}$}}
 
+bool simple_if_return_return_expr(int i) {
+  if (i > 10)
+return true;
+  return i < 5;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool simple_if_return_return_expr(int i) {{{$}}
+// CHECK-FIXES: {{^}}  return i > 10 ||  i < 5;{{$}}
+// CHECK-FIXES: {{^}$}}
+
+bool simple_if_return_return_expr_negated(int i) {
+  if (i > 10)
+return false;
+  return i > 5;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool simple_if_return_return_expr_negated(int i) {{{$}}
+// CHECK-FIXES: {{^}}  return i <= 10 &&  i > 5;{{$}}
+// CHECK-FIXES: {{^}$}}
+
 bool if_implicit_bool_expr(int i) {
   if (i & 1) {
 return true;
Index: clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-case.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-case.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-case.cpp
@@ -143,6 +143,20 @@
 return false;
 return true;
 
+  case 18:
+if (j > 10)
+  return true;
+return j < 5;
+// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: {{.*}} in conditional return
+// CHECK-FIXES: return j > 10 ||  j < 5;
+
+  case 19:
+if (j > 10)
+  return false;
+return j > 5;
+// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: {{.*}} in conditional return
+// CHECK-FIXES: return j <= 10 &&  j > 5;
+
   case 100: {
 if (b == true)
   j = 10;
Index: clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
@@ -24,10 +24,14 @@
 ``if (false) t(); else f()

[PATCH] D131858: [clang] Track the templated entity in type substitution.

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3748-3751
+  // If it is a template, import all related things.
+  if (Error Err = ImportTemplateInformation(D, ToFunction))
+return std::move(Err);
+

What's the reason of moving this hunk?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D133102: [clang-tidy] Extend simplify-boolean-expr check

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 457228.
njames93 added a comment.

Remove unnecessary includes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133102

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-case.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr.cpp
@@ -357,10 +357,6 @@
   if (i == j) return (i == 0); else return false;
 }
 
-bool conditional_return_statements_else_expr(int i, int j) {
-  if (i == j) return true; else return (i == 0);
-}
-
 bool negated_conditional_return_statements(int i) {
   if (i == 0) return false; else return true;
 }
@@ -387,6 +383,46 @@
 // CHECK-FIXES-NEXT: {{^}}  return i == 1;{{$}}
 // CHECK-FIXES-NEXT: {{^}$}}
 
+bool getFallback();
+
+bool conditional_return_complex(bool B) {
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: return B ||  getFallback();
+  if (B)
+return true;
+  else
+return getFallback();
+}
+
+bool conditional_return_complex_compound(bool B) {
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: return B ||  getFallback();
+  if (B) {
+return true;
+  } else {
+return getFallback();
+  }
+}
+
+bool conditional_return_complex_compound_negated(bool B) {
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: return !B &&  getFallback();
+  if (B) {
+return false;
+  } else {
+return getFallback();
+  }
+}
+
+bool conditional_return_complex_negated(bool B) {
+  // CHECK-MESSAGES: :[[@LINE+2]]:3: warning: {{.*}} in conditional return statement
+  // CHECK-FIXES: return !B &&  getFallback();
+  if (B)
+return false;
+  else
+return getFallback();
+}
+
 bool negated_conditional_compound_return_statements(int i) {
   if (i == 1) {
 return false;
@@ -745,6 +781,26 @@
 // CHECK-FIXES: {{^  return i <= 10;$}}
 // CHECK-FIXES: {{^}$}}
 
+bool simple_if_return_return_expr(int i) {
+  if (i > 10)
+return true;
+  return i < 5;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool simple_if_return_return_expr(int i) {{{$}}
+// CHECK-FIXES: {{^}}  return i > 10 ||  i < 5;{{$}}
+// CHECK-FIXES: {{^}$}}
+
+bool simple_if_return_return_expr_negated(int i) {
+  if (i > 10)
+return false;
+  return i > 5;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:3: warning: {{.*}} in conditional return
+// CHECK-FIXES: {{^}}bool simple_if_return_return_expr_negated(int i) {{{$}}
+// CHECK-FIXES: {{^}}  return i <= 10 &&  i > 5;{{$}}
+// CHECK-FIXES: {{^}$}}
+
 bool if_implicit_bool_expr(int i) {
   if (i & 1) {
 return true;
Index: clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-case.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-case.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-case.cpp
@@ -143,6 +143,20 @@
 return false;
 return true;
 
+  case 18:
+if (j > 10)
+  return true;
+return j < 5;
+// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: {{.*}} in conditional return
+// CHECK-FIXES: return j > 10 ||  j < 5;
+
+  case 19:
+if (j > 10)
+  return false;
+return j > 5;
+// CHECK-MESSAGES: :[[@LINE-2]]:5: warning: {{.*}} in conditional return
+// CHECK-FIXES: return j <= 10 &&  j > 5;
+
   case 100: {
 if (b == true)
   j = 10;
Index: clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/simplify-boolean-expr.rst
@@ -24,10 +24,14 @@
 ``if (false) t(); else f();``  ``f();``
 ``if (e) return true; else return false;`` ``return e;``
 ``if (e) return false; else return true;`` ``return !e;``
+``if (e) return true; else return x;`` ``return e || x;``
+``if (e) return false; else return x;````return !e && x;``
 ``if (e) b = true; else b = false;``   ``b = e;``
 ``if (e) b = false; else b = true;``   ``b = !e;``
 ``if (e) ret

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

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong 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/D132109/new/

https://reviews.llvm.org/D132109

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

[PATCH] D131858: [clang] Track the templated entity in type substitution.

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3748-3751
+  // If it is a template, import all related things.
+  if (Error Err = ImportTemplateInformation(D, ToFunction))
+return std::move(Err);
+

martong wrote:
> What's the reason of moving this hunk?
All the changes here to the ASTImporter revolve around the need to import the 
template bits before the type, as now the type itself can depend on the 
templated declaration, and this would cause a cycle otherwise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D133029: [Sema] Allow to diagnose the references to std::vector with incomplete T

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D133029#3763120 , @shafik wrote:

> Do you have an idea of how common this might be?

I don't have the numbers yet, but this broke the builds in C++20 mode inside 
the core libraries we are using after libc++ change 

 that makes vector `constexpr`.
I suspect every medium-to-large codebase will have a few instances.
One common pattern that I have seen goes something like:

  // foo.h
  struct Inner;
  class Outer {
std::vector elements;
  private:
size_t size() { return elements.size(); }
void some_other_method();
  };
  
  struct Inner {
Outer* outer;
  };
  
  // foo.cpp
  Outer::some_other_method() { /* uses members that would actually fail with 
incomplete type */ }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133029

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

[PATCH] D132756: [clang][dataflow] Refactor `TypeErasedDataflowAnalysisTest` - replace usage of the deprecated overload of `checkDataflow`.

[weiyi via Phabricator via cfe-commits]

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

Update build target dependency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132756

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
  utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel

Index: utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel
===
--- utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel
+++ utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel
@@ -134,6 +134,7 @@
 "//clang:serialization",
 "//clang:tooling",
 "//llvm:Support",
+"//llvm:TestingADT",
 "//llvm:TestingSupport",
 "//llvm:gmock",
 "//llvm:gtest",
Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -24,8 +24,10 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Testing/ADT/StringMapEntry.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -42,12 +44,10 @@
 using namespace dataflow;
 using namespace test;
 using namespace ast_matchers;
-using ::testing::_;
-using ::testing::ElementsAre;
+using llvm::IsStringMapEntry;
+using ::testing::DescribeMatcher;
 using ::testing::IsEmpty;
-using ::testing::IsNull;
 using ::testing::NotNull;
-using ::testing::Pair;
 using ::testing::Test;
 using ::testing::UnorderedElementsAre;
 
@@ -129,7 +129,8 @@
 }
 
 struct FunctionCallLattice {
-  llvm::SmallSet CalledFunctions;
+  using FunctionSet = llvm::SmallSet;
+  FunctionSet CalledFunctions;
 
   bool operator==(const FunctionCallLattice &Other) const {
 return CalledFunctions == Other.CalledFunctions;
@@ -195,16 +196,20 @@
 
 ASSERT_THAT_ERROR(
 test::checkDataflow(
-Code, "target",
-[](ASTContext &C, Environment &) {
-  return FunctionCallAnalysis(C);
-},
+AnalysisInputs(
+Code, ast_matchers::hasName("target"),
+[](ASTContext &C, Environment &) {
+  return FunctionCallAnalysis(C);
+})
+.withASTBuildArgs({"-fsyntax-only", "-std=c++17"})
+.withASTBuildVirtualMappedFiles(std::move(FilesContents)),
+/*VerifyResults=*/
 [&Expectations](
-llvm::ArrayRef>>
-Results,
-ASTContext &) { EXPECT_THAT(Results, Expectations); },
-{"-fsyntax-only", "-std=c++17"}, FilesContents),
+const llvm::StringMap<
+DataflowAnalysisState> &Results,
+const AnalysisOutputs &) {
+  EXPECT_THAT(Results, Expectations);
+}),
 llvm::Succeeded());
   }
 };
@@ -212,12 +217,16 @@
 MATCHER_P(HoldsFunctionCallLattice, m,
   ((negation ? "doesn't hold" : "holds") +
llvm::StringRef(" a lattice element that ") +
-   ::testing::DescribeMatcher(m, negation))
+   DescribeMatcher(m))
   .str()) {
   return ExplainMatchResult(m, arg.Lattice, result_listener);
 }
 
-MATCHER_P(HasCalledFunctions, m, "") {
+MATCHER_P(HasCalledFunctions, m,
+  ((negation ? "doesn't hold" : "holds") +
+   llvm::StringRef(" a set of called functions that ") +
+   DescribeMatcher(m))
+  .str()) {
   return ExplainMatchResult(m, arg.CalledFunctions, result_listener);
 }
 
@@ -231,9 +240,9 @@
   // [[p]]
 }
   )";
-  runDataflow(Code, UnorderedElementsAre(
-Pair("p", HoldsFunctionCallLattice(HasCalledFunctions(
-  UnorderedElementsAre("foo", "bar"));
+  runDataflow(Code, UnorderedElementsAre(IsStringMapEntry(
+"p", HoldsFunctionCallLattice(HasCalledFunctions(
+ UnorderedElementsAre("foo", "bar"));
 }
 
 TEST_F(NoreturnDestructorTest, ConditionalOperatorLeftBranchReturns) {
@@ -246,9 +255,9 @@
   // [[p]]
 }
   )";
-  runDataflow(Code, UnorderedElementsAre(
-Pair("p", HoldsFunctionCallLattice(HasCalledFunctions(
-  UnorderedElementsAre("foo"));
+  runDataflow(Code, UnorderedElementsAre(IsStringMapEntry(
+"p", HoldsFunctionCallLattice(HasCalledFunctions(
+ UnorderedElementsAre("foo"));
 }

[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added inline comments.



Comment at: clang/test/Sema/constant-conversion.c:30
+  s.b = 1; // one-bit-warning {{implicit truncation from 'int' to a 
one-bit wide bit-field changes value from 1 to -1}}
+  s.b = true;  // no-warning (we suppress it manually to reduce false 
positives)
+  s.b = false; // no-warning

(Sorry for being late to the party, with post commit comments. I'm just trying 
to understand the reasoning about always suppressing the warning based on 
"true".)

Isn't the code a bit suspicious when using true/false from stdbool.h, while 
using a signed bitfield?

When doing things like 
```
(s.b == true) ? 1 : 0
```
the result would always be zero if s.b is a signed 1-bit bitfield.

So wouldn't it make more sense to actually use an unsigned bitfield (such as 
the bool type from stdbool.h) when the intent is to store a boolean value and 
using defines from stdbool.h?

Is perhaps the idea that we will get warnings about `(s.b == true)` being 
redundant in situations like this, and then we do not need a warning on the 
bitfield assignment? Such a reasoning would actually make some sense, since 
`(s.b == true)` never would be true even when the bitfield is assigned a 
non-constant value, so we can't rely on catching the problem by only looking at 
bitfield assignments involving true/false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132851

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

[clang] 0cb0809 - [Clang][Comments] Parse `` in doc comments correctly

[Egor Zhdan via cfe-commits]

Author: Egor Zhdan
Date: 2022-09-01T13:17:42+01:00
New Revision: 0cb08093365478224f2a58cece42527fb8e1199e

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

LOG: [Clang][Comments] Parse `` in doc comments correctly

This is a valid HTML5 tag. Previously it triggered a Clang error (`HTML start 
tag prematurely ended, expected attribute name or '>'`) since Clang was 
treating `/>` as a text token. This was happening because after lexing the 
closing quote (`"`) the lexer state was reset to "Normal" while the tag was not 
actually closed yet: `>` was not yet parsed at that point.

rdar://91464292

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

Added: 


Modified: 
clang/lib/AST/CommentLexer.cpp
clang/test/Index/comment-to-html-xml-conversion.cpp
clang/test/Sema/warn-documentation.cpp

Removed: 




diff  --git a/clang/lib/AST/CommentLexer.cpp b/clang/lib/AST/CommentLexer.cpp
index 61ce8979f13f5..f0250fc9fd55e 100644
--- a/clang/lib/AST/CommentLexer.cpp
+++ b/clang/lib/AST/CommentLexer.cpp
@@ -701,7 +701,7 @@ void Lexer::lexHTMLStartTag(Token &T) {
 
   C = *BufferPtr;
   if (!isHTMLIdentifierStartingCharacter(C) &&
-  C != '=' && C != '\"' && C != '\'' && C != '>') {
+  C != '=' && C != '\"' && C != '\'' && C != '>' && C != '/') {
 State = LS_Normal;
 return;
   }

diff  --git a/clang/test/Index/comment-to-html-xml-conversion.cpp 
b/clang/test/Index/comment-to-html-xml-conversion.cpp
index 1fedd382365cf..ec49e5af31da1 100644
--- a/clang/test/Index/comment-to-html-xml-conversion.cpp
+++ b/clang/test/Index/comment-to-html-xml-conversion.cpp
@@ -744,6 +744,26 @@ void comment_to_html_conversion_37();
 // CHECK-NEXT: (CXComment_Text Text=[ ] IsWhitespace)
 // CHECK-NEXT: (CXComment_InlineCommand CommandName=[anchor] 
RenderAnchor Arg[0]=A)))]
 
+/// Aaa bbb
+void comment_to_html_conversion_38();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: 
FunctionDecl=comment_to_html_conversion_38:{{.*}} FullCommentAsHTML=[ Aaa bbb] FullCommentAsXML=[comment_to_html_conversion_38c:@F@comment_to_html_conversion_38#void
 comment_to_html_conversion_38() Aaa 
bbb]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa bbb])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] SelfClosing)
+
+/// Aaa ccc
+void comment_to_html_conversion_39();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: 
FunctionDecl=comment_to_html_conversion_39:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_39c:@F@comment_to_html_conversion_39#void
 comment_to_html_conversion_39() Aaa 
ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] SelfClosing)
+
 /// Aaa ccc
 void comment_to_html_conversion_40();
 
@@ -754,6 +774,36 @@ void comment_to_html_conversion_40();
 // CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
 // CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] Attrs: src=)
 
+/// Aaa ccc
+void comment_to_html_conversion_41();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: 
FunctionDecl=comment_to_html_conversion_41:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_41c:@F@comment_to_html_conversion_41#void
 comment_to_html_conversion_41() Aaa 
ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] Attrs: src=path)
+
+/// Aaa ccc
+void comment_to_html_conversion_42();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: 
FunctionDecl=comment_to_html_conversion_42:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_42c:@F@comment_to_html_conversion_42#void
 comment_to_html_conversion_42() Aaa 
ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] Attrs: src=path 
SelfClosing)
+
+/// Aaa ddd
+void comment_to_html_conversion_43();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: 
FunctionDecl=comment_to_html_conversion_43:{{.*}} FullCommentAsHTML=[ Aaa ddd] FullCommentAsXML=[comment_to_html_conversion_43c:@F@comment_to_html_conversion_43#void
 comment_to_html_conversion_43() Aaa 
ddd]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// 

[PATCH] D132932: [Clang][Comments] Parse `` in doc comments correctly

[Egor Zhdan 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 rG0cb080933654: [Clang][Comments] Parse `` in doc comments correctly (authored by egorzhdan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132932

Files:
  clang/lib/AST/CommentLexer.cpp
  clang/test/Index/comment-to-html-xml-conversion.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -62,6 +62,21 @@
 /// 
 int test_html11(int);
 
+/// Aaa bbb
+int test_html12(int);
+
+/// Aaa bbb
+int test_html13(int);
+
+/// Aaa bbb
+int test_html14(int);
+
+/// Aaa bbb
+int test_html15(int);
+
+/// Aaa bbb
+int test_html16(int);
+
 /// Meow
 int test_html_nesting1(int);
 
Index: clang/test/Index/comment-to-html-xml-conversion.cpp
===
--- clang/test/Index/comment-to-html-xml-conversion.cpp
+++ clang/test/Index/comment-to-html-xml-conversion.cpp
@@ -744,6 +744,26 @@
 // CHECK-NEXT: (CXComment_Text Text=[ ] IsWhitespace)
 // CHECK-NEXT: (CXComment_InlineCommand CommandName=[anchor] RenderAnchor Arg[0]=A)))]
 
+/// Aaa bbb
+void comment_to_html_conversion_38();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_38:{{.*}} FullCommentAsHTML=[ Aaa bbb] FullCommentAsXML=[comment_to_html_conversion_38c:@F@comment_to_html_conversion_38#void comment_to_html_conversion_38() Aaa bbb]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa bbb])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] SelfClosing)
+
+/// Aaa ccc
+void comment_to_html_conversion_39();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_39:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_39c:@F@comment_to_html_conversion_39#void comment_to_html_conversion_39() Aaa ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] SelfClosing)
+
 /// Aaa ccc
 void comment_to_html_conversion_40();
 
@@ -754,6 +774,36 @@
 // CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
 // CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] Attrs: src=)
 
+/// Aaa ccc
+void comment_to_html_conversion_41();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_41:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_41c:@F@comment_to_html_conversion_41#void comment_to_html_conversion_41() Aaa ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] Attrs: src=path)
+
+/// Aaa ccc
+void comment_to_html_conversion_42();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_42:{{.*}} FullCommentAsHTML=[ Aaa ccc] FullCommentAsXML=[comment_to_html_conversion_42c:@F@comment_to_html_conversion_42#void comment_to_html_conversion_42() Aaa ccc]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ccc])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] Attrs: src=path SelfClosing)
+
+/// Aaa ddd
+void comment_to_html_conversion_43();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_43:{{.*}} FullCommentAsHTML=[ Aaa ddd] FullCommentAsXML=[comment_to_html_conversion_43c:@F@comment_to_html_conversion_43#void comment_to_html_conversion_43() Aaa ddd]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ Aaa ddd])
+// CHECK-NEXT: (CXComment_HTMLStartTag Name=[img] Attrs: src= SelfClosing)
+
 /// Aaa.
 class comment_to_xml_conversion_01 {
 // CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-1]]:7: ClassDecl=comment_to_xml_conversion_01:{{.*}} FullCommentAsXML=[comment_to_xml_conversion_01c:@S@comment_to_xml_conversion_01class comment_to_xml_conversion_01 {} Aaa.]
Index: clang/lib/AST/CommentLexer.cpp
===
--- clang/lib/AST/CommentLexer.cpp
+++ clang/lib/AST/CommentLexer.cpp
@@ -701,7 +701,7 @@
 
   C = *Bu

[PATCH] D133009: [libclang] Fix conversion from `StringRef` to `CXString`

[Egor Zhdan via Phabricator via cfe-commits]

egorzhdan added inline comments.



Comment at: clang/test/Index/comment-lots-of-unknown-commands.c:3
 
+// XFAIL: *
+

aaronpuchert wrote:
> egorzhdan wrote:
> > gribozavr2 wrote:
> > > egorzhdan wrote:
> > > > This test was never properly passing. Because of the bug in string 
> > > > conversion, the printed comments contained the entire source file and 
> > > > not just the comments' text, which was enough to cause `// CHECK`-s in 
> > > > the test to succeed.
> > > > ```
> > > > // CHECK: (CXComment_InlineCommand CommandName=[tel] 
> > > > RenderNormal HasTrailingNewline)
> > > > // CHECK: (CXComment_InlineCommand CommandName=[n] RenderNormal 
> > > > HasTrailingNewline))
> > > > // CHECK:   (CXComment_VerbatimLine 
> > > > Text=[\n@Lo\n@il\n@tle\n@axt\n@ba\n@ust\n@ac\n@tpe\n@tpl\n@ctG\n@ru\n@m\n@tG\n@it\n@rh\n@G\n@rpc\n@el\n@er\n@w\n@eo\n@tx\n@oo\n@dD\n@dD\n*/\nvoid
> > > >  f();\n\n// CHECK:  CommentAST=[\n// CHECK:
> > > > (CXComment_FullComment\n// CHECK:   (CXComment_Paragraph\n// CHECK: 
> > > >  ...
> > > > ```
> > > Please update the test to pass then. Here's the diff:
> > > 
> > > ```
> > > diff --git a/clang/test/Index/comment-lots-of-unknown-commands.c 
> > > b/clang/test/Index/comment-lots-of-unknown-commands.c
> > > index 41a03d394488..e1adcc150b1e 100644
> > > --- a/clang/test/Index/comment-lots-of-unknown-commands.c
> > > +++ b/clang/test/Index/comment-lots-of-unknown-commands.c
> > > @@ -1,6 +1,5 @@
> > >  // RUN: c-index-test -test-load-source-reparse 1 local %s | FileCheck %s
> > > 
> > > -// XFAIL: *
> > > 
> > >  // See PR 21254. We had too few bits to encode command IDs so if you 
> > > created
> > >  // enough of them the ID codes would wrap around. This test creates 
> > > commands up
> > > @@ -183,7 +182,7 @@ void f();
> > >  // CHECK: (CXComment_InlineCommand CommandName=[ei] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[oun] 
> > > RenderNormal HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[ou] RenderNormal 
> > > HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[nl] RenderNormal 
> > > HasTrailingNewline)
> > > +// CHECK: (CXComment_InlineCommand CommandName=[n] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[ien] 
> > > RenderNormal HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[fr] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[en] RenderNormal 
> > > HasTrailingNewline)
> > > @@ -204,7 +203,7 @@ void f();
> > >  // CHECK: (CXComment_InlineCommand CommandName=[fro] 
> > > RenderNormal HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[ast] 
> > > RenderNormal HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[ae] RenderNormal 
> > > HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[nN] RenderNormal 
> > > HasTrailingNewline)
> > > +// CHECK: (CXComment_InlineCommand CommandName=[n] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[pc] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[tae] 
> > > RenderNormal HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[ws] RenderNormal 
> > > HasTrailingNewline)
> > > @@ -268,10 +267,8 @@ void f();
> > >  // CHECK: (CXComment_InlineCommand CommandName=[an] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[de] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[tel] 
> > > RenderNormal HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[nd] RenderNormal 
> > > HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[dic] 
> > > RenderNormal HasTrailingNewline)
> > > +// CHECK: (CXComment_InlineCommand CommandName=[n] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[Lo] RenderNormal 
> > > HasTrailingNewline)
> > > -// CHECK: (CXComment_InlineCommand CommandName=[il] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[tle] 
> > > RenderNormal HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[axt] 
> > > RenderNormal HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineCommand CommandName=[ba] RenderNormal 
> > > HasTrailingNewline)
> > > @@ -283,7 +280,6 @@ void f();
> > >  // CHECK: (CXComment_InlineCommand CommandName=[ru] RenderNormal 
> > > HasTrailingNewline)
> > >  // CHECK: (CXComment_InlineComma

[PATCH] D133105: [Clang][Comments] Fix `Index/comment-lots-of-unknown-commands.c`

[Egor Zhdan via Phabricator via cfe-commits]

egorzhdan created this revision.
egorzhdan added a reviewer: aaronpuchert.
Herald added a subscriber: arphaman.
Herald added a project: All.
egorzhdan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This re-enables a test after it was disabled in 
https://reviews.llvm.org/D133009.

Fixes #57484.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133105

Files:
  clang/test/Index/comment-lots-of-unknown-commands.c


Index: clang/test/Index/comment-lots-of-unknown-commands.c
===
--- clang/test/Index/comment-lots-of-unknown-commands.c
+++ clang/test/Index/comment-lots-of-unknown-commands.c
@@ -1,7 +1,5 @@
 // RUN: c-index-test -test-load-source-reparse 1 local %s | FileCheck %s
 
-// XFAIL: *
-
 // See PR 21254. We had too few bits to encode command IDs so if you created
 // enough of them the ID codes would wrap around. This test creates commands up
 // to an ID of 258. Ideally we should check for large numbers, but that would
@@ -37,7 +35,7 @@
 @ei
 @oun
 @ou
-@nl
+@nlnl
 @ien
 @fr
 @en
@@ -58,7 +56,7 @@
 @fro
 @ast
 @ae
-@nN
+@nNnN
 @pc
 @tae
 @ws
@@ -122,10 +120,10 @@
 @an
 @de
 @tel
-@nd
-@dic
+@ndnd
+@dict
 @Lo
-@il
+@ilil
 @tle
 @axt
 @ba
@@ -137,7 +135,7 @@
 @ru
 @m
 @tG
-@it
+@itit
 @rh
 @G
 @rpc
@@ -183,7 +181,7 @@
 // CHECK: (CXComment_InlineCommand CommandName=[ei] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[oun] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[ou] RenderNormal 
HasTrailingNewline)
-// CHECK: (CXComment_InlineCommand CommandName=[nl] RenderNormal 
HasTrailingNewline)
+// CHECK: (CXComment_InlineCommand CommandName=[nlnl] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[ien] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[fr] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[en] RenderNormal 
HasTrailingNewline)
@@ -204,7 +202,7 @@
 // CHECK: (CXComment_InlineCommand CommandName=[fro] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[ast] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[ae] RenderNormal 
HasTrailingNewline)
-// CHECK: (CXComment_InlineCommand CommandName=[nN] RenderNormal 
HasTrailingNewline)
+// CHECK: (CXComment_InlineCommand CommandName=[nNnN] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[pc] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[tae] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[ws] RenderNormal 
HasTrailingNewline)
@@ -268,10 +266,10 @@
 // CHECK: (CXComment_InlineCommand CommandName=[an] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[de] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[tel] RenderNormal 
HasTrailingNewline)
-// CHECK: (CXComment_InlineCommand CommandName=[nd] RenderNormal 
HasTrailingNewline)
-// CHECK: (CXComment_InlineCommand CommandName=[dic] RenderNormal 
HasTrailingNewline)
+// CHECK: (CXComment_InlineCommand CommandName=[ndnd] RenderNormal 
HasTrailingNewline)
+// CHECK: (CXComment_InlineCommand CommandName=[dict] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[Lo] RenderNormal 
HasTrailingNewline)
-// CHECK: (CXComment_InlineCommand CommandName=[il] RenderNormal 
HasTrailingNewline)
+// CHECK: (CXComment_InlineCommand CommandName=[ilil] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[tle] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[axt] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[ba] RenderNormal 
HasTrailingNewline)
@@ -283,7 +281,7 @@
 // CHECK: (CXComment_InlineCommand CommandName=[ru] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[m] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[tG] RenderNormal 
HasTrailingNewline)
-// CHECK: (CXComment_InlineCommand CommandName=[it] RenderNormal 
HasTrailingNewline)
+// CHECK: (CXComment_InlineCommand CommandName=[itit] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[rh] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[G] RenderNormal 
HasTrailingNewline)
 // CHECK: (CXComment_InlineCommand CommandName=[rpc] RenderNormal 
HasTrailingNewline)


Index: clang/test/I

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3228
+
+  Results = completions(R"objc(
+  Fo^

nit: just `completions("Fo^", ...)`



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3274
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+  EXPECT_THAT(C, ElementsAre(AllOf(named("Food"),
+   kind(CompletionItemKind::Interface;

we actually need to issue the new request in a context that can show 
`FoodClass`, as ATM if we were to do filtering during speculative query 
response, this test would still pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132962

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

[clang] e0746a8 - [clang] cleanup -fstrict-flex-arrays implementation

[via cfe-commits]

Author: serge-sans-paille
Date: 2022-09-01T15:06:21+02:00
New Revision: e0746a8a8d647ccee41accf16df895c9b03552c2

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

LOG: [clang] cleanup -fstrict-flex-arrays implementation

This is a follow up to https://reviews.llvm.org/D126864, addressing some 
remaining
comments.

It also considers union with a single zero-length array field as FAM for each
value of -fstrict-flex-arrays.

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CGExpr.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGen/bounds-checking-fam.c
clang/test/CodeGen/object-size-flex-array.c

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index ee1012efc086d..d921ea5d5da99 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1143,7 +1143,7 @@ def fallow_unsupported : Flag<["-"], 
"fallow-unsupported">, Group;
 def fapple_kext : Flag<["-"], "fapple-kext">, Group, 
Flags<[CC1Option]>,
   HelpText<"Use Apple's kernel extensions ABI">,
   MarshallingInfoFlag>;
-def fstrict_flex_arrays_EQ : Joined<["-"], 
"fstrict-flex-arrays=">,Group,
+def fstrict_flex_arrays_EQ : Joined<["-"], "fstrict-flex-arrays=">, 
Group,
   MetaVarName<"">, Values<"0,1,2">,
   LangOpts<"StrictFlexArrays">,
   Flags<[CC1Option]>,

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index e26822eaa601e..324eb56167b56 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -906,10 +906,8 @@ static bool isFlexibleArrayMemberExpr(const Expr *E,
 if (const auto *FD = dyn_cast(ME->getMemberDecl())) {
   // FIXME: Sema doesn't treat a T[1] union member as a flexible array
   // member, only a T[0] or T[] member gets that treatment.
-  // Under StrictFlexArraysLevel, obey c99+ that disallows FAM in union, 
see
-  // C11 6.7.2.1 §18
   if (FD->getParent()->isUnion())
-return StrictFlexArraysLevel < 2;
+return true;
   RecordDecl::field_iterator FI(
   DeclContext::decl_iterator(const_cast(FD)));
   return ++FI == FD->getParent()->field_end();

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 03b0c72fac9b6..897ab701493c2 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -15888,16 +15888,10 @@ static bool IsTailPaddedMemberArray(Sema &S, const 
llvm::APInt &Size,
   if (!ND)
 return false;
 
-  if (StrictFlexArraysLevel >= 2 && Size != 0)
-return false;
-
-  if (StrictFlexArraysLevel == 1 && Size.uge(2))
-return false;
-
   // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing
   // arrays to be treated as flexible-array-members, we still emit diagnostics
   // as if they are not. Pending further discussion...
-  if (StrictFlexArraysLevel == 0 && Size != 1)
+  if (StrictFlexArraysLevel >= 2 || Size.uge(2))
 return false;
 
   const FieldDecl *FD = dyn_cast(ND);
@@ -16065,8 +16059,8 @@ void Sema::CheckArrayAccess(const Expr *BaseExpr, const 
Expr *IndexExpr,
 if (BaseType->isIncompleteType())
   return;
 
-// FIXME: this check should belong to the IsTailPaddedMemberArray call
-// below.
+// FIXME: this check should be used to set IsUnboundedArray from the
+// beginning.
 llvm::APInt size = ArrayTy->getSize();
 if (!size.isStrictlyPositive())
   return;

diff  --git a/clang/test/CodeGen/bounds-checking-fam.c 
b/clang/test/CodeGen/bounds-checking-fam.c
index 8a96c261992a5..1b4f183bee12b 100644
--- a/clang/test/CodeGen/bounds-checking-fam.c
+++ b/clang/test/CodeGen/bounds-checking-fam.c
@@ -35,6 +35,51 @@ int test_two(struct Two *p, int i) {
   return p->a[i] + (p->a)[i];
 }
 
+union uZero {
+  int a[0];
+};
+union uOne {
+  int a[1];
+};
+union uTwo {
+  int a[2];
+};
+union uThree {
+  int a[3];
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_uzero{{.*}}(
+int test_uzero(union uZero *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  // CHECK-STRICT-1-NOT: @__ubsan
+  // CHECK-STRICT-2-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_uone{{.*}}(
+int test_uone(union uOne *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  // CHECK-STRICT-1-NOT: @__ubsan
+  // CHECK-STRICT-2: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_utwo{{.*}}(
+int test_utwo(union uTwo *p, int i) {
+  // CHECK-STRICT-0: @__ubsan
+  // CHECK-STRICT-1: @__ubsan
+  // CHECK-STRICT-2: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_uthree{{.*}}(
+int test_uthree(union uThree *p, int

[PATCH] D132944: [clang] cleanup -fstrict-flex-arrays implementation

[serge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe0746a8a8d64: [clang] cleanup -fstrict-flex-arrays 
implementation (authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132944

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/bounds-checking-fam.c
  clang/test/CodeGen/object-size-flex-array.c

Index: clang/test/CodeGen/object-size-flex-array.c
===
--- clang/test/CodeGen/object-size-flex-array.c
+++ clang/test/CodeGen/object-size-flex-array.c
@@ -24,7 +24,7 @@
   double c[2];
 } foo2_t;
 
-// CHECK-LABEL: @bar
+// CHECK-LABEL: @bar(
 unsigned bar(foo_t *f) {
   // CHECK-STRICT-0: ret i32 %
   // CHECK-STRICT-1: ret i32 %
@@ -32,7 +32,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @bar0
+// CHECK-LABEL: @bar0(
 unsigned bar0(foo0_t *f) {
   // CHECK-STRICT-0: ret i32 %
   // CHECK-STRICT-1: ret i32 %
@@ -40,7 +40,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @bar1
+// CHECK-LABEL: @bar1(
 unsigned bar1(foo1_t *f) {
   // CHECK-STRICT-0: ret i32 %
   // CHECK-STRICT-1: ret i32 %
@@ -48,7 +48,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @bar2
+// CHECK-LABEL: @bar2(
 unsigned bar2(foo2_t *f) {
   // CHECK-STRICT-0: ret i32 %
   // CHECK-STRICT-1: ret i32 16
@@ -73,7 +73,7 @@
   float f;
 } foofoo2_t;
 
-// CHECK-LABEL: @babar0
+// CHECK-LABEL: @babar0(
 unsigned babar0(foofoo0_t *f) {
   // CHECK-STRICT-0: ret i32 0
   // CHECK-STRICT-1: ret i32 0
@@ -81,7 +81,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @babar1
+// CHECK-LABEL: @babar1(
 unsigned babar1(foofoo1_t *f) {
   // CHECK-STRICT-0: ret i32 8
   // CHECK-STRICT-1: ret i32 8
@@ -89,7 +89,7 @@
   return OBJECT_SIZE_BUILTIN(f->c, 1);
 }
 
-// CHECK-LABEL: @babar2
+// CHECK-LABEL: @babar2(
 unsigned babar2(foofoo2_t *f) {
   // CHECK-STRICT-0: ret i32 16
   // CHECK-STRICT-1: ret i32 16
Index: clang/test/CodeGen/bounds-checking-fam.c
===
--- clang/test/CodeGen/bounds-checking-fam.c
+++ clang/test/CodeGen/bounds-checking-fam.c
@@ -35,6 +35,51 @@
   return p->a[i] + (p->a)[i];
 }
 
+union uZero {
+  int a[0];
+};
+union uOne {
+  int a[1];
+};
+union uTwo {
+  int a[2];
+};
+union uThree {
+  int a[3];
+};
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_uzero{{.*}}(
+int test_uzero(union uZero *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  // CHECK-STRICT-1-NOT: @__ubsan
+  // CHECK-STRICT-2-NOT: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_uone{{.*}}(
+int test_uone(union uOne *p, int i) {
+  // CHECK-STRICT-0-NOT: @__ubsan
+  // CHECK-STRICT-1-NOT: @__ubsan
+  // CHECK-STRICT-2: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_utwo{{.*}}(
+int test_utwo(union uTwo *p, int i) {
+  // CHECK-STRICT-0: @__ubsan
+  // CHECK-STRICT-1: @__ubsan
+  // CHECK-STRICT-2: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
+// CHECK-LABEL: define {{.*}} @{{.*}}test_uthree{{.*}}(
+int test_uthree(union uThree *p, int i) {
+  // CHECK-STRICT-0: @__ubsan
+  // CHECK-STRICT-1: @__ubsan
+  // CHECK-STRICT-2: @__ubsan
+  return p->a[i] + (p->a)[i];
+}
+
 // CHECK-LABEL: define {{.*}} @{{.*}}test_three{{.*}}(
 int test_three(struct Three *p, int i) {
   // CHECK-STRICT-0: call void @__ubsan_handle_out_of_bounds_abort(
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15888,16 +15888,10 @@
   if (!ND)
 return false;
 
-  if (StrictFlexArraysLevel >= 2 && Size != 0)
-return false;
-
-  if (StrictFlexArraysLevel == 1 && Size.uge(2))
-return false;
-
   // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing
   // arrays to be treated as flexible-array-members, we still emit diagnostics
   // as if they are not. Pending further discussion...
-  if (StrictFlexArraysLevel == 0 && Size != 1)
+  if (StrictFlexArraysLevel >= 2 || Size.uge(2))
 return false;
 
   const FieldDecl *FD = dyn_cast(ND);
@@ -16065,8 +16059,8 @@
 if (BaseType->isIncompleteType())
   return;
 
-// FIXME: this check should belong to the IsTailPaddedMemberArray call
-// below.
+// FIXME: this check should be used to set IsUnboundedArray from the
+// beginning.
 llvm::APInt size = ArrayTy->getSize();
 if (!size.isStrictlyPositive())
   return;
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -906,10 +906,8 @@
 if (const auto *FD = dyn_cast(ME->getMemberDecl())) {

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I don't mind the idea, but wonder if some level of RFC/project-wide decision 
should be made here?  WDYT @aaron.ballman ?




Comment at: clang/lib/Driver/Driver.cpp:5425
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
-  if (CCGenDiagnostics && A) {
-SmallString<128> CrashDirectory(A->getValue());
+  const char *CrashDirectory = CCGenDiagnostics && A
+   ? A->getValue()

`StringRef` would be better here instead, which should mean you don't have to 
create the SmallString below, and could just work in `Twine`s for everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133108: [clang] Rework IsTailPaddedMemberArray into isFlexibleArrayMemberExpr

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added a reviewer: jyknight.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes a bunch of FIXME within IsTailPaddedMemberArray related code, no
functional change expected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133108

Files:
  clang/lib/Sema/SemaChecking.cpp


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15877,17 +15877,24 @@
 << TRange << Op->getSourceRange();
 }
 
-/// Check whether this array fits the idiom of a size-one tail padded
-/// array member of a struct.
+/// Check whether this array fits the idiom of a flexible array member,
+/// depending on -fstrict-flex-array value
 ///
-/// We avoid emitting out-of-bounds access warnings for such arrays as they are
-/// commonly used to emulate flexible arrays in C89 code.
-static bool IsTailPaddedMemberArray(Sema &S, const llvm::APInt &Size,
-const NamedDecl *ND,
-unsigned StrictFlexArraysLevel) {
+/// We avoid emitting out-of-bounds access warnings for such arrays.
+static bool isFlexibleArrayMemberExpr(Sema &S, const Expr *E,
+  const NamedDecl *ND,
+  unsigned StrictFlexArraysLevel) {
+
   if (!ND)
 return false;
 
+  const ConstantArrayType *ArrayTy =
+  S.Context.getAsConstantArrayType(E->getType());
+  llvm::APInt Size = ArrayTy->getSize();
+
+  if (Size == 0)
+return true;
+
   // FIXME: While the default -fstrict-flex-arrays=0 permits Size>1 trailing
   // arrays to be treated as flexible-array-members, we still emit diagnostics
   // as if they are not. Pending further discussion...
@@ -15953,9 +15960,19 @@
   const ConstantArrayType *ArrayTy =
   Context.getAsConstantArrayType(BaseExpr->getType());
 
+  const unsigned StrictFlexArraysLevel = getLangOpts().StrictFlexArrays;
+
+  const NamedDecl *ND = nullptr;
+  if (const DeclRefExpr *DRE = dyn_cast(BaseExpr))
+ND = DRE->getDecl();
+  if (const MemberExpr *ME = dyn_cast(BaseExpr))
+ND = ME->getMemberDecl();
+
   const Type *BaseType =
   ArrayTy == nullptr ? nullptr : ArrayTy->getElementType().getTypePtr();
-  bool IsUnboundedArray = (BaseType == nullptr);
+  bool IsUnboundedArray =
+  (BaseType == nullptr) ||
+  isFlexibleArrayMemberExpr(*this, BaseExpr, ND, StrictFlexArraysLevel);
   if (EffectiveType->isDependentType() ||
   (!IsUnboundedArray && BaseType->isDependentType()))
 return;
@@ -15970,12 +15987,6 @@
 index = -index;
   }
 
-  const NamedDecl *ND = nullptr;
-  if (const DeclRefExpr *DRE = dyn_cast(BaseExpr))
-ND = DRE->getDecl();
-  if (const MemberExpr *ME = dyn_cast(BaseExpr))
-ND = ME->getMemberDecl();
-
   if (IsUnboundedArray) {
 if (EffectiveType->isFunctionType())
   return;
@@ -16053,17 +16064,10 @@
 // example). In this case we have no information about whether the array
 // access exceeds the array bounds. However we can still diagnose an array
 // access which precedes the array bounds.
-//
-// FIXME: this check should be redundant with the IsUnboundedArray check
-// above.
 if (BaseType->isIncompleteType())
   return;
 
-// FIXME: this check should be used to set IsUnboundedArray from the
-// beginning.
 llvm::APInt size = ArrayTy->getSize();
-if (!size.isStrictlyPositive())
-  return;
 
 if (BaseType != EffectiveType) {
   // Make sure we're comparing apples to apples when comparing index to 
size
@@ -16093,11 +16097,6 @@
 if (AllowOnePastEnd ? index.ule(size) : index.ult(size))
   return;
 
-// Also don't warn for Flexible Array Member emulation.
-const unsigned StrictFlexArraysLevel = getLangOpts().StrictFlexArrays;
-if (IsTailPaddedMemberArray(*this, size, ND, StrictFlexArraysLevel))
-  return;
-
 // Suppress the warning if the subscript expression (as identified by the
 // ']' location) and the index expression are both from macro expansions
 // within a system header.


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -15877,17 +15877,24 @@
 << TRange << Op->getSourceRange();
 }
 
-/// Check whether this array fits the idiom of a size-one tail padded
-/// array member of a struct.
+/// Check whether this array fits the idiom of a flexible array member,
+/// depending on -fstrict-flex-array value
 ///
-/// We avoid emitting out-of-bounds access warnings for such arrays as they are
-/// commonly used to emulate flexible arrays in C89 code.
-static bool IsTailPaddedMemberArray(Sema &S, const llvm::AP

[PATCH] D133072: [clang] fix profiling of template arguments of template and declaration kind

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Generally happy here.  Two quick suggestions, otherewise LGTM.




Comment at: clang/lib/AST/ASTContext.cpp:5119
 // Find the insert position again.
-DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos);
+[[maybe_unused]] auto *Nothing =
+DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, 
InsertPos);

Instead of `maybe_unused` put this and the assert inside of a `#if NDEBUG` so 
we don't do the `Nothing` work.  Same below.



Comment at: clang/lib/Sema/SemaTemplate.cpp:5817
   // fall back to just producing individual arguments.
-  Converted.insert(Converted.end(),
-   ArgumentPack.begin(), ArgumentPack.end());
+  for (const TemplateArgument &I : ArgumentPack)
+Converted.push_back(Context.getCanonicalTemplateArgument(I));

Ooh, thats a transform!  

`llvm::transform(ArgumentPack, std::back_inserter(Converted), [](const auto &I) 
{ return Context.getCanonicalTemplateArgument(I));`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133072

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

[PATCH] D129488: [Sema] Delay evaluation of std::source_location::current() in default arguments

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D129488#3762490 , @jyknight wrote:

> The more I look at this, the more I feel like, if all the above behaviors are 
> correct, source_location::current() simply shouldn't be marked consteval at 
> all. If it was constexpr, instead, it'd work great _without_ needing any 
> special-casing, because the "must evaluate immediately upon seeing function 
> definition even in default arg position" wouldn't apply to it. And that's the 
> behavior we //want// from it!

I think it's true that this would give the desired behavior without any hacks 
on the side of the compiler.
I am not sure why `consteval` was chosen in the first place, I suspect the goal 
was to reduce runtime costs.

I prototyped a change that forces `constexpr` on `current()` and it definitely 
works wrt to the scope rules.
If we choose this approach, Clang will not be standard-compliant with C++20. 
Raising this with WG21 is definitely something we need to do.
@aaron.ballman would you be able to discuss this within the WG21 or provide 
pointers on how these things are done? I am a total stranger to this process.

In the meantime, what options do we have for existing C++20 Standard and 
existing STL libraries?
The ones I see are:

1. Clang keeps the status quo and provides wrong results for `consteval 
source_location::current()`.
2. Clang internally switches "hacks up" `current()` to be `constexpr` even if 
it was marked as consteval, which means it can produce runtime calls and 
silently compile code where using `consteval` function `current()` would be an 
error.
3. Same as (2), but try to mitigate incompatibilities, e.g. lower calls to 
`current()` to constants, do not do the codegen the body of `current()`.
4. Keep `current()` consteval, but special-case calls to it.

(1) does not look like an option.
I am not sure entirely various problems that (2) and (3) would entail, but this 
only seems reasonable if WG21 decides `current()` must be changed to 
`constexpr` in the long run.
A compiler change for (2) is very simple, so I would vote for this if there are 
not problematic implications for users, e.g. what are the compatibility 
guarantees when linking together code compiled both with Clang and GCC? Would 
be break any promises Clang has around those?
If we are to choose between (3) and (4), I would definitely go with (4) as the 
compiler changes are roughly equivalent, but treating `current()` as 
`consteval` function produces exactly the diagnostics you expect from 
`consteval` (e.g. prevents taking a pointer to it).

I'd say we should choose between (2) and (4).
Prefer (2) if WG21 decides to switch to `constexpr` or we are certain there are 
no potential issues (see comment about GCC compatibility above).
Prefer (4) otherwise.

In D129488#3763252 , @ChuanqiXu wrote:

> In D129488#3760982 , @ilya-biryukov 
> wrote:
>
>> @aaron.ballman, that's my reading of the standard as well. Do you think we 
>> should proceed with the current approach or is there another direction worth 
>> pursuing to make source_location work?
>>
>> In D129488#3760178 , @ChuanqiXu 
>> wrote:
>>
>>> 
>
> It is just an example. I just wanted to say we could solve more problems if 
> we chose that way. The issue of the module may not so painful and it 
> shouldn't be hard to handle it specially. (Although specially handling looks 
> not good, this is the reason why I didn't fix it). But after all, I am not 
> objecting (nor approving) this for the modules example.

As mentioned earlier, I also think this might be the right call.
I just wanted to mention it's a shift against the current Clang design and 
exposes a few other issues that need to be worked out and also makes processing 
default arguments slower.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129488

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

[clang] db898d4 - [clang][dataflow] Refactor `TestingSupport.h`

[Wei Yi Tee via cfe-commits]

Author: Wei Yi Tee
Date: 2022-09-01T13:21:34Z
New Revision: db898d43b08e13f5c1fda92b8341cd1709f5af21

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

LOG: [clang][dataflow] Refactor `TestingSupport.h`

- Add `AnalysisInputs` struct as the parameters for `checkDataflow`, and 
renamed `AnalysisData` struct to `AnalysisOutputs` which contains the data 
structures generated from a dataflow analysis run.

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

Reviewed By: gribozavr2, sgatev

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

Added: 


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

Removed: 




diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
index 7c50453abe509..56df21617e5a3 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
@@ -52,6 +52,22 @@ isAnnotationDirectlyAfterStatement(const Stmt *Stmt, 
unsigned AnnotationBegin,
   return true;
 }
 
+llvm::DenseMap
+test::getAnnotationLinesAndContent(const AnalysisOutputs &AO) {
+  llvm::DenseMap LineNumberToContent;
+  auto Code = AO.Code.code();
+  auto Annotations = AO.Code.ranges();
+  auto &SM = AO.ASTCtx.getSourceManager();
+  for (auto &AnnotationRange : Annotations) {
+auto LineNumber =
+SM.getPresumedLineNumber(SM.getLocForStartOfFile(SM.getMainFileID())
+ .getLocWithOffset(AnnotationRange.Begin));
+auto Content = Code.slice(AnnotationRange.Begin, 
AnnotationRange.End).str();
+LineNumberToContent[LineNumber] = Content;
+  }
+  return LineNumberToContent;
+}
+
 llvm::Expected>
 test::buildStatementToAnnotationMapping(const FunctionDecl *Func,
 llvm::Annotations AnnotatedCode) {

diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index cdcf61ee22577..a23b0ec0709a3 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -56,47 +56,160 @@ std::ostream &operator<<(std::ostream &OS,
 
 namespace test {
 
-// Returns assertions based on annotations that are present after statements in
-// `AnnotatedCode`.
-llvm::Expected>
-buildStatementToAnnotationMapping(const FunctionDecl *Func,
-  llvm::Annotations AnnotatedCode);
-
-struct AnalysisData {
+/// Contains data structures required and produced by a dataflow analysis run.
+struct AnalysisOutputs {
+  /// Input code that is analyzed. Points within the code may be marked with
+  /// annotations to facilitate testing.
+  ///
+  /// Example:
+  /// void target(int *x) {
+  ///   *x; // [[p]]
+  /// }
+  /// From the annotation `p`, the line number and analysis state immediately
+  /// after the statement `*x` can be retrieved and verified.
+  llvm::Annotations Code;
+  /// AST context generated from `Code`.
   ASTContext &ASTCtx;
+  /// The function whose body is analyzed.
+  const FunctionDecl *Target;
+  /// Contains the control flow graph built from the body of the `Target`
+  /// function and is analyzed.
   const ControlFlowContext &CFCtx;
-  const Environment &Env;
+  /// The analysis to be run.
   TypeErasedDataflowAnalysis &Analysis;
-  llvm::DenseMap &Annotations;
-  std::vector> &BlockStates;
+  /// Initial state to start the analysis.
+  const Environment &InitEnv;
+  // Stores the state of a CFG block if it has been evaluated by the analysis.
+  // The indices correspond to the block IDs.
+  llvm::ArrayRef> BlockStates;
+};
+
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisInputs {
+  /// Required fields are set in constructor.
+  AnalysisInputs(
+  llvm::StringRef CodeArg,
+  ast_matchers::internal::Matcher TargetFuncMatcherArg,
+  std::function MakeAnalysisArg)
+  : Code(CodeArg), TargetFuncMatcher(std::move(TargetFuncMatcherArg

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

[weiyi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdb898d43b08e: [clang][dataflow] Refactor `TestingSupport.h` 
(authored by wyt).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132147

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

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1234,49 +1234,48 @@
   template 
   T Make();
 )");
-const tooling::FileContentMappings FileContents(Headers.begin(),
-Headers.end());
 UncheckedOptionalAccessModelOptions Options{
 /*IgnoreSmartPointerDereference=*/true};
 std::vector Diagnostics;
 llvm::Error Error = checkDataflow(
-SourceCode, FuncMatcher,
-[Options](ASTContext &Ctx, Environment &) {
-  return UncheckedOptionalAccessModel(Ctx, Options);
-},
-[&Diagnostics, Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
-ASTContext &Ctx, const CFGStmt &Stmt,
-const TypeErasedDataflowAnalysisState &State) mutable {
-  auto StmtDiagnostics =
-  Diagnoser.diagnose(Ctx, Stmt.getStmt(), State.Env);
-  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
-},
-[&Diagnostics](AnalysisData AnalysisData) {
-  auto &SrcMgr = AnalysisData.ASTCtx.getSourceManager();
-
+AnalysisInputs(
+SourceCode, std::move(FuncMatcher),
+[Options](ASTContext &Ctx, Environment &) {
+  return UncheckedOptionalAccessModel(Ctx, Options);
+})
+.withPostVisitCFG(
+[&Diagnostics,
+ Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+ASTContext &Ctx, const CFGElement &Elt,
+const TypeErasedDataflowAnalysisState &State) mutable {
+  auto Stmt = Elt.getAs();
+  if (!Stmt) {
+return;
+  }
+  auto StmtDiagnostics =
+  Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env);
+  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
+})
+.withASTBuildArgs(
+{"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"})
+.withASTBuildVirtualMappedFiles(
+tooling::FileContentMappings(Headers.begin(), Headers.end())),
+/*VerifyResults=*/[&Diagnostics](
+  const llvm::DenseMap
+  &Annotations,
+  const AnalysisOutputs &AO) {
   llvm::DenseSet AnnotationLines;
-  for (const auto &Pair : AnalysisData.Annotations) {
-auto *Stmt = Pair.getFirst();
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getBeginLoc()));
-// We add both the begin and end locations, so that if the
-// statement spans multiple lines then the test will fail.
-//
-// FIXME: Going forward, we should change this to instead just
-// get the single line number from the annotation itself, rather
-// than looking at the statement it's attached to.
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getEndLoc()));
+  for (const auto &[Line, _] : Annotations) {
+AnnotationLines.insert(Line);
   }
-
+  auto &SrcMgr = AO.ASTCtx.getSourceManager();
   llvm::DenseSet DiagnosticLines;
   for (SourceLocation &Loc : Diagnostics) {
 DiagnosticLines.insert(SrcMgr.getPresumedLineNumber(Loc));
   }
 
   EXPECT_THAT(DiagnosticLines, ContainerEq(AnnotationLines));
-},
-{"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}, FileContents);
+});
 if (Error)
   FAIL() << llvm::toString(std::move(Error));
   }
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -56,47 +56,160 @@
 
 namespace test {
 
-// Returns assertions based on annotations that are present after statements in
-// `AnnotatedCode`.
-llvm::Expected>
-buildStatementToAnnotationMapping(const FunctionDecl *Func,
-  llvm::

[PATCH] D131858: [clang] Track the templated entity in type substitution.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Just did a quick scroll through this (as it is quite large!), but the general 
idea seems like a fine one to me.  I AM concerned about how it interacts with 
the deferred concepts instantiation that I've been working on 
(https://reviews.llvm.org/D126907), particularly around the MLTAL work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D132689: [Object] Refactor code for extracting offload binaries

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132689

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

[PATCH] D133072: [clang] fix profiling of template arguments of template and declaration kind

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:5119
 // Find the insert position again.
-DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos);
+[[maybe_unused]] auto *Nothing =
+DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, 
InsertPos);

erichkeane wrote:
> Instead of `maybe_unused` put this and the assert inside of a `#if NDEBUG` so 
> we don't do the `Nothing` work.  Same below.
We still need to do the `FindNodeOrInsertPos` bit, to refresh the insert 
position.
So I think this would end up too noisy to add two sets of blocks.



Comment at: clang/lib/Sema/SemaTemplate.cpp:5817
   // fall back to just producing individual arguments.
-  Converted.insert(Converted.end(),
-   ArgumentPack.begin(), ArgumentPack.end());
+  for (const TemplateArgument &I : ArgumentPack)
+Converted.push_back(Context.getCanonicalTemplateArgument(I));

erichkeane wrote:
> Ooh, thats a transform!  
> 
> `llvm::transform(ArgumentPack, std::back_inserter(Converted), [](const auto 
> &I) { return Context.getCanonicalTemplateArgument(I));`
That is true, though unless we can come up with a clearer spelling for that, it 
looks less readable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133072

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

[PATCH] D133072: [clang] fix profiling of template arguments of template and declaration kind

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:5119
 // Find the insert position again.
-DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos);
+[[maybe_unused]] auto *Nothing =
+DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, 
InsertPos);

mizvekov wrote:
> erichkeane wrote:
> > Instead of `maybe_unused` put this and the assert inside of a `#if NDEBUG` 
> > so we don't do the `Nothing` work.  Same below.
> We still need to do the `FindNodeOrInsertPos` bit, to refresh the insert 
> position.
> So I think this would end up too noisy to add two sets of blocks.
Ah! I didn't realize we were counting on the side-effects here.  Thanks!



Comment at: clang/lib/Sema/SemaTemplate.cpp:5817
   // fall back to just producing individual arguments.
-  Converted.insert(Converted.end(),
-   ArgumentPack.begin(), ArgumentPack.end());
+  for (const TemplateArgument &I : ArgumentPack)
+Converted.push_back(Context.getCanonicalTemplateArgument(I));

mizvekov wrote:
> erichkeane wrote:
> > Ooh, thats a transform!  
> > 
> > `llvm::transform(ArgumentPack, std::back_inserter(Converted), [](const auto 
> > &I) { return Context.getCanonicalTemplateArgument(I));`
> That is true, though unless we can come up with a clearer spelling for that, 
> it looks less readable to me.
But I like transform :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133072

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

[PATCH] D131858: [clang] Track the templated entity in type substitution.

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D131858#3763851 , @erichkeane 
wrote:

> Just did a quick scroll through this (as it is quite large!), but the general 
> idea seems like a fine one to me.  I AM concerned about how it interacts with 
> the deferred concepts instantiation that I've been working on 
> (https://reviews.llvm.org/D126907), particularly around the MLTAL work.

I think I did rebase it on top of that at one point, thought it was short lived 
as it was reverted if I am not mistaken.
But I can certainly do it again if you merge yours first, and I am available 
any time to help if it happens the other way around.

But the gist of the change is simple, you will simply have to pass in the 
templated declaration for every level that you push into the MLTAL.
I think that should be the only change that affects you, besides possibly churn 
in AST tests if you plan to have any.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D133072: [clang] fix profiling of template arguments of template and declaration kind

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:5817
   // fall back to just producing individual arguments.
-  Converted.insert(Converted.end(),
-   ArgumentPack.begin(), ArgumentPack.end());
+  for (const TemplateArgument &I : ArgumentPack)
+Converted.push_back(Context.getCanonicalTemplateArgument(I));

erichkeane wrote:
> mizvekov wrote:
> > erichkeane wrote:
> > > Ooh, thats a transform!  
> > > 
> > > `llvm::transform(ArgumentPack, std::back_inserter(Converted), [](const 
> > > auto &I) { return Context.getCanonicalTemplateArgument(I));`
> > That is true, though unless we can come up with a clearer spelling for 
> > that, it looks less readable to me.
> But I like transform :(
I will think of something, even if we have to add a new helper :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133072

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

[PATCH] D131858: [clang] Track the templated entity in type substitution.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131858#3763878 , @mizvekov wrote:

> In D131858#3763851 , @erichkeane 
> wrote:
>
>> Just did a quick scroll through this (as it is quite large!), but the 
>> general idea seems like a fine one to me.  I AM concerned about how it 
>> interacts with the deferred concepts instantiation that I've been working on 
>> (https://reviews.llvm.org/D126907), particularly around the MLTAL work.
>
> I think I did rebase it on top of that at one point, thought it was short 
> lived as it was reverted if I am not mistaken.
> But I can certainly do it again if you merge yours first, and I am available 
> any time to help if it happens the other way around.
>
> But the gist of the change is simple, you will simply have to pass in the 
> templated declaration for every level that you push into the MLTAL.
> I think that should be the only change that affects you, besides possibly 
> churn in AST tests if you plan to have any.

Part of the problem is that the concepts instantiation needs to reform the 
MLTAL AFTER the fact using `Sema::getTemplateInstantiationArgs` and 
`addInstantiatedParametersToScope`.  BUT I don't se the corresponding changes 
on quick look here.

As far as which goes first, I obviously have my preference, since I'm about 10 
months into that patch now :/  I'm still working through a couple of issues 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

[Orlando Cazalet-Hyams via Phabricator via cfe-commits]

Orlando added subscribers: probinson, Orlando.
Orlando added a comment.

When constructing the compiler command line (here in Clang.cpp 
)
 there's a special case for SCE debugger tuning:

  // -gdwarf-aranges turns on the emission of the aranges section in the
  // backend.
  // Always enabled for SCE tuning.
  bool NeedAranges = DebuggerTuning == llvm::DebuggerKind::SCE;

This isn't an area I've looked at before but It looks like we (Sony - cc 
@probinson), at least historically, have DWARF consumers that want to see 
`.debug_aranges`. Given there's this bit of code for the compiler command, IMO 
it would be reasonable to add the special case for the linker command too. 
Please could you add that?

I mentioned on the discourse thread 

 but will repeat here for other reviewers: This approach SGTM but I would feel 
more comfortable if someone with more experience reviewed this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133092

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

[PATCH] D131858: [clang] Track the templated entity in type substitution.

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D131858#3763892 , @erichkeane 
wrote:

> In D131858#3763878 , @mizvekov 
> wrote:
>
>> In D131858#3763851 , @erichkeane 
>> wrote:
>>
>>> Just did a quick scroll through this (as it is quite large!), but the 
>>> general idea seems like a fine one to me.  I AM concerned about how it 
>>> interacts with the deferred concepts instantiation that I've been working 
>>> on (https://reviews.llvm.org/D126907), particularly around the MLTAL work.
>>
>> I think I did rebase it on top of that at one point, thought it was short 
>> lived as it was reverted if I am not mistaken.
>> But I can certainly do it again if you merge yours first, and I am available 
>> any time to help if it happens the other way around.
>>
>> But the gist of the change is simple, you will simply have to pass in the 
>> templated declaration for every level that you push into the MLTAL.
>> I think that should be the only change that affects you, besides possibly 
>> churn in AST tests if you plan to have any.
>
> Part of the problem is that the concepts instantiation needs to reform the 
> MLTAL AFTER the fact using `Sema::getTemplateInstantiationArgs` and 
> `addInstantiatedParametersToScope`.  BUT I don't se the corresponding changes 
> on quick look here.
>
> As far as which goes first, I obviously have my preference, since I'm about 
> 10 months into that patch now :/  I'm still working through a couple of 
> issues though.

`getTemplateInstantiationArgs` is updated in this patch to build the MLTAL with 
the new information.

I don' have any changes to `addInstantiatedParametersToScope`, as that doesn't 
modify MLTAL, it just forwards it to SubstType.

No worries about who merges first, you are racing @rsmith here, you will win 
easily :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131858

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

[clang] 5a4aece - [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

[Wei Yi Tee via cfe-commits]

Author: Wei Yi Tee
Date: 2022-09-01T13:48:29Z
New Revision: 5a4aece76de7c73b9adf29b0c3f4e8641575cee1

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

LOG: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

Moves the work required for retrieving annotation states into the `SetupTest` 
and `PostVisitCFG` callback to avoid having to run a separate pass over the CFG 
after analysis has completed.

Reviewed By: gribozavr2, sgatev, ymandel

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

Added: 


Modified: 
clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Removed: 




diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
index 56df21617e5a..c0c05b0efdbb 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
@@ -53,11 +53,11 @@ isAnnotationDirectlyAfterStatement(const Stmt *Stmt, 
unsigned AnnotationBegin,
 }
 
 llvm::DenseMap
-test::getAnnotationLinesAndContent(const AnalysisOutputs &AO) {
+test::buildLineToAnnotationMapping(SourceManager &SM,
+   llvm::Annotations AnnotatedCode) {
   llvm::DenseMap LineNumberToContent;
-  auto Code = AO.Code.code();
-  auto Annotations = AO.Code.ranges();
-  auto &SM = AO.ASTCtx.getSourceManager();
+  auto Code = AnnotatedCode.code();
+  auto Annotations = AnnotatedCode.ranges();
   for (auto &AnnotationRange : Annotations) {
 auto LineNumber =
 SM.getPresumedLineNumber(SM.getLocForStartOfFile(SM.getMainFileID())

diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index a23b0ec0709a..285c0c991d23 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -96,6 +96,11 @@ template  struct AnalysisInputs {
 
   /// Optional fields can be set with methods of the form `withFieldName(...)`.
   AnalysisInputs &&
+  withSetupTest(std::function Arg) && {
+SetupTest = std::move(Arg);
+return std::move(*this);
+  }
+  AnalysisInputs &&
   withPostVisitCFG(std::function
Arg) && {
@@ -120,6 +125,11 @@ template  struct AnalysisInputs {
   /// takes as argument the AST generated from the code being analyzed and the
   /// initial state from which the analysis starts with.
   std::function MakeAnalysis;
+  /// Optional. If provided, this function is executed immediately before
+  /// running the dataflow analysis to allow for additional setup. All fields 
in
+  /// the `AnalysisOutputs` argument will be initialized except for the
+  /// `BlockStates` field which is only computed later during the analysis.
+  std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
   std::function>
 buildStatementToAnnotationMapping(const FunctionDecl *Func,
   llvm::Annotations AnnotatedCode);
 
-/// Returns line numbers and content of the annotations in `AO.Code`.
+/// Returns line numbers and content of the annotations in `AnnotatedCode`.
 llvm::DenseMap
-getAnnotationLinesAndContent(const AnalysisOutputs &AO);
-
-// FIXME: Return a string map instead of a vector of pairs.
-//
-/// Returns the analysis states at each annotated statement in `AO.Code`.
-template 
-llvm::Expected>>>
-getAnnotationStates(const AnalysisOutputs &AO) {
-  using StateT = DataflowAnalysisState;
-  // FIXME: Extend to annotations on non-statement constructs.
-  // Get annotated statements.
-  llvm::Expected>
-  MaybeStmtToAnnotations =
-  buildStatementToAnnotationMapping(AO.Target, AO.Code);
-  if (!MaybeStmtToAnnotations)
-return MaybeStmtToAnnotations.takeError();
-  auto &StmtToAnnotations = *MaybeStmtToAnnotations;
-
-  // Compute a map from statement annotations to the state computed
-  // for the program point immediately after the annotated statement.
-  std::vector> Results;
-  for (const CFGBlock *Block : AO.CFCtx.getCFG()) {
-// Skip blocks that were not evaluated.
-if (!AO.BlockStates[Block->getBlockID()])
-  continue;
-
-transferBlock(
-AO.CFCtx, AO.BlockStates, *Block, AO.InitEnv, AO.Analysis,
-[&Results,
- &StmtToAnnotations](const clang::CFGElement &Element,
- const TypeErasedDataflowAnalysisState &State) {
-  auto Stmt = Element.getAs();
-  if (!Stmt)
-return;
-  auto It = StmtToAnnotations.find(Stmt->getStmt());
-  if (It == StmtToAnnotations.end())
-

[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

[weiyi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5a4aece76de7: [clang][dataflow] Add `SetupTest` parameter 
for `AnalysisInputs`. (authored by wyt).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132377

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

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -96,6 +96,11 @@
 
   /// Optional fields can be set with methods of the form `withFieldName(...)`.
   AnalysisInputs &&
+  withSetupTest(std::function Arg) && {
+SetupTest = std::move(Arg);
+return std::move(*this);
+  }
+  AnalysisInputs &&
   withPostVisitCFG(std::function
Arg) && {
@@ -120,6 +125,11 @@
   /// takes as argument the AST generated from the code being analyzed and the
   /// initial state from which the analysis starts with.
   std::function MakeAnalysis;
+  /// Optional. If provided, this function is executed immediately before
+  /// running the dataflow analysis to allow for additional setup. All fields in
+  /// the `AnalysisOutputs` argument will be initialized except for the
+  /// `BlockStates` field which is only computed later during the analysis.
+  std::function SetupTest = nullptr;
   /// Optional. If provided, this function is applied on each CFG element after
   /// the analysis has been run.
   std::function
-getAnnotationLinesAndContent(const AnalysisOutputs &AO);
-
-// FIXME: Return a string map instead of a vector of pairs.
-//
-/// Returns the analysis states at each annotated statement in `AO.Code`.
-template 
-llvm::Expected>>>
-getAnnotationStates(const AnalysisOutputs &AO) {
-  using StateT = DataflowAnalysisState;
-  // FIXME: Extend to annotations on non-statement constructs.
-  // Get annotated statements.
-  llvm::Expected>
-  MaybeStmtToAnnotations =
-  buildStatementToAnnotationMapping(AO.Target, AO.Code);
-  if (!MaybeStmtToAnnotations)
-return MaybeStmtToAnnotations.takeError();
-  auto &StmtToAnnotations = *MaybeStmtToAnnotations;
-
-  // Compute a map from statement annotations to the state computed
-  // for the program point immediately after the annotated statement.
-  std::vector> Results;
-  for (const CFGBlock *Block : AO.CFCtx.getCFG()) {
-// Skip blocks that were not evaluated.
-if (!AO.BlockStates[Block->getBlockID()])
-  continue;
-
-transferBlock(
-AO.CFCtx, AO.BlockStates, *Block, AO.InitEnv, AO.Analysis,
-[&Results,
- &StmtToAnnotations](const clang::CFGElement &Element,
- const TypeErasedDataflowAnalysisState &State) {
-  auto Stmt = Element.getAs();
-  if (!Stmt)
-return;
-  auto It = StmtToAnnotations.find(Stmt->getStmt());
-  if (It == StmtToAnnotations.end())
-return;
-  auto *Lattice =
-  llvm::any_cast(&State.Lattice.Value);
-  Results.emplace_back(It->second, StateT{*Lattice, State.Env});
-});
-  }
-
-  return Results;
-}
+buildLineToAnnotationMapping(SourceManager &SM,
+ llvm::Annotations AnnotatedCode);
 
 /// Runs dataflow specified from `AI.MakeAnalysis` and `AI.PostVisitCFG` on the
 /// body of the function that matches `AI.TargetFuncMatcher` in `AI.Code`.
@@ -200,9 +166,9 @@
 ///
 ///   `VerifyResults` must be provided.
 template 
-llvm::Error checkDataflow(
-AnalysisInputs AI,
-std::function VerifyResults) {
+llvm::Error
+checkDataflow(AnalysisInputs AI,
+  std::function VerifyResults) {
   // Build AST context from code.
   llvm::Annotations AnnotatedCode(AI.Code);
   auto Unit = tooling::buildASTFromCodeWithArgs(
@@ -236,7 +202,7 @@
 return MaybeCFCtx.takeError();
   auto &CFCtx = *MaybeCFCtx;
 
-  // Initialize states and run dataflow analysis.
+  // Initialize states for running dataflow analysis.
   DataflowAnalysisContext DACtx(std::make_unique());
   Environment InitEnv(DACtx, *Target);
   auto Analysis = AI.MakeAnalysis(Context, InitEnv);
@@ -251,19 +217,26 @@
 };
   }
 
-  // If successful, the run returns a mapping from block IDs to the
-  // post-analysis states for the CFG blocks that have been evaluated.
+  // Additional test setup.
+  AnalysisOutputs AO{AnnotatedCode, Context, Target, CFCtx,
+ Analysis,  InitEnv, {}};
+  if (AI.SetupTest) {
+if (auto Error = AI.SetupTest(AO))
+  return Error;
+  }
+
+  // If successful, the dataflow analysis returns a mapping from block IDs to
+  // the post-analysis states for the CFG blocks that have been evaluated.
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnaly

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:7671
+ AllArgs, CallType))
+return true;
+

rjmccall wrote:
> yihanaa wrote:
> > rjmccall wrote:
> > > You can just pull the argument expressions out of the `CallExpr`; you 
> > > don't need to call `GatherArgumentsForCall`.
> > > You can just pull the argument expressions out of the `CallExpr`; you 
> > > don't need to call `GatherArgumentsForCall`.
> > 
> > This GatherArgumentsForCall  was used to do the common sema checking and 
> > emit warning, like './main.cpp:5:40: warning: passing 'volatile char *' to 
> > parameter of type 'const void *' discards qualifiers 
> > [-Wincompatible-pointer-types-discards-qualifiers]' hahaha, for this is a 
> > common case, I also think  GatherArgumentsForCall is not a good choice
> > , so I try to find a replacement, e.g. ImpCastExprToType or other ways, 
> > what do you think about?
> `convertArgumentToType` should trigger any useful warnings in the second and 
> third arguments.  For the first, I don't actually think there are any 
> warnings we care about.
I'm sorry John, I can't find `convertArgumentToType ` in clang, did you mean 
`ConvertArgumentsForCall` or `ImpCastExprToType`. we can't use 
`ConvertArgumentsForCall `, because `ConvertArgumentsForCall ` has checked if 
current CallExpr calling a builtin function with custom sema checking, it will 
do nothing and return.



Comment at: clang/lib/Sema/SemaChecking.cpp:7684
+  if (FirstArgResult.isInvalid())
+return true;
+

rjmccall wrote:
> There is in fact a `DefaultFunctionArrayLvalueConversion` method you can use 
> to do all of this at once, and you don't need to explicitly check for a 
> function or array type first.
+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

[David Spickett via Phabricator via cfe-commits]

DavidSpickett created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Fixes #57486

These pre v4 architectures are not specifically supported
by codegen. As demonstrated in the linked issue.

This removes the options and associated testing.

The Pre_v4 build attribute remains mainly because its absence
would be more confusing. It will not be used other than to
complete the list of build attributes as shown in the ABI.

https://github.com/ARM-software/abi-aa/blob/main/addenda32/addenda32.rst#3352the-target-related-attributes


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133109

Files:
  clang/docs/ReleaseNotes.rst
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/test/MC/ARM/directive-arch-armv2.s
  llvm/test/MC/ARM/directive-arch-armv2a.s
  llvm/test/MC/ARM/directive-arch-armv3.s
  llvm/test/MC/ARM/directive-arch-armv3m.s
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -20,21 +20,20 @@
 
 namespace {
 const char *ARMArch[] = {
-"armv2",   "armv2a", "armv3",   "armv3m","armv4",
-"armv4t",  "armv5",  "armv5t",  "armv5e","armv5te",
-"armv5tej","armv6",  "armv6j",  "armv6k","armv6hl",
-"armv6t2", "armv6kz","armv6z",  "armv6zk",   "armv6-m",
-"armv6m",  "armv6sm","armv6s-m","armv7-a",   "armv7",
-"armv7a",  "armv7ve","armv7hl", "armv7l","armv7-r",
-"armv7r",  "armv7-m","armv7m",  "armv7k","armv7s",
-"armv7e-m","armv7em","armv8-a", "armv8", "armv8a",
-"armv8l",  "armv8.1-a",  "armv8.1a","armv8.2-a", "armv8.2a",
-"armv8.3-a",   "armv8.3a",   "armv8.4-a",   "armv8.4a",  "armv8.5-a",
-"armv8.5a","armv8.6-a",  "armv8.6a","armv8.7-a", "armv8.7a",
-"armv8.8-a",   "armv8.8a",   "armv8-r", "armv8r","armv8-m.base",
-"armv8m.base", "armv8-m.main",   "armv8m.main", "iwmmxt","iwmmxt2",
-"xscale",  "armv8.1-m.main", "armv9-a", "armv9", "armv9a",
-"armv9.1-a",   "armv9.1a",   "armv9.2-a",   "armv9.2a",
+"armv4","armv4t",  "armv5",  "armv5t",  "armv5e",
+"armv5te",  "armv5tej","armv6",  "armv6j",  "armv6k",
+"armv6hl",  "armv6t2", "armv6kz","armv6z",  "armv6zk",
+"armv6-m",  "armv6m",  "armv6sm","armv6s-m","armv7-a",
+"armv7","armv7a",  "armv7ve","armv7hl", "armv7l",
+"armv7-r",  "armv7r",  "armv7-m","armv7m",  "armv7k",
+"armv7s",   "armv7e-m","armv7em","armv8-a", "armv8",
+"armv8a",   "armv8l",  "armv8.1-a",  "armv8.1a","armv8.2-a",
+"armv8.2a", "armv8.3-a",   "armv8.3a",   "armv8.4-a",   "armv8.4a",
+"armv8.5-a","armv8.5a","armv8.6-a",  "armv8.6a","armv8.7-a",
+"armv8.7a", "armv8.8-a",   "armv8.8a",   "armv8-r", "armv8r",
+"armv8-m.base", "armv8m.base", "armv8-m.main",   "armv8m.main", "iwmmxt",
+"iwmmxt2",  "xscale",  "armv8.1-m.main", "armv9-a", "armv9",
+"armv9a",   "armv9.1-a",   "armv9.1a",   "armv9.2-a",   "armv9.2a",
 };
 
 template 
@@ -438,14 +437,6 @@
 }
 
 TEST(TargetParserTest, testARMArch) {
-  EXPECT_TRUE(
-  testARMArch("armv2", "generic", "v2", ARMBuildAttrs::CPUArch::Pre_v4));
-  EXPECT_TRUE(
-  testARMArch("armv2a", "generic", "v2a", ARMBuildAttrs::CPUArch::Pre_v4));
-  EXPECT_TRUE(
-  testARMArch("armv3", "generic", "v3", ARMBuildAttrs::CPUArch::Pre_v4));
-  EXPECT_TRUE(
-  testARMArch("armv3m", "generic", "v3m", ARMBuildAttrs::CPUArch::Pre_v4));
   EXPECT_TRUE(
   testARMArch("armv4", "strongarm", "v4",
   ARMBuildAttrs::CPUArch::v4));
@@ -603,10 +594,6 @@
   EXPECT_FALSE(testARMExtension("xscale", ARM::ArchKind::INVALID, "crc"));
   EXPECT_FALSE(testARMExtension("swift", ARM::ArchKind::INVALID, "crc"));
 
-  EXPECT_FALSE(testARMExtension("generic", ARM::ArchKind::ARMV2, "thumb"));
-  EXPECT_FALSE(testARMExtension("generic", ARM::ArchKind::ARMV2A, "thumb"));
-  EXPECT_FALSE(testARMExtension("generic", ARM::ArchKind::ARMV3, "thumb"));
-  EXPECT_FALSE(testARMExtension("generic", ARM::ArchKind::ARMV3M, "thumb"));
   EXPECT_FALSE(testARMExtension("generic", ARM::ArchKind::ARMV4, "dsp"));
   EXPECT_FALSE(testARMExtension("

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133109

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

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 457257.
dgoldman added a comment.

Fix strings + test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132962

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/TestIndex.cpp
  clang-tools-extra/clangd/unittests/TestIndex.h

Index: clang-tools-extra/clangd/unittests/TestIndex.h
===
--- clang-tools-extra/clangd/unittests/TestIndex.h
+++ clang-tools-extra/clangd/unittests/TestIndex.h
@@ -39,6 +39,8 @@
llvm::StringRef USRPrefix);
 // Create an @interface or @implementation.
 Symbol objcClass(llvm::StringRef Name);
+// Create an @interface or @implementation category.
+Symbol objcCategory(llvm::StringRef Name, llvm::StringRef CategoryName);
 // Create an @protocol.
 Symbol objcProtocol(llvm::StringRef Name);
 
Index: clang-tools-extra/clangd/unittests/TestIndex.cpp
===
--- clang-tools-extra/clangd/unittests/TestIndex.cpp
+++ clang-tools-extra/clangd/unittests/TestIndex.cpp
@@ -99,6 +99,11 @@
   return objcSym(Name, index::SymbolKind::Class, "objc(cs)");
 }
 
+Symbol objcCategory(llvm::StringRef Name, llvm::StringRef CategoryName) {
+  std::string USRPrefix = ("objc(cy)" + Name + "@").str();
+  return objcSym(CategoryName, index::SymbolKind::Extension, USRPrefix);
+}
+
 Symbol objcProtocol(llvm::StringRef Name) {
   return objcSym(Name, index::SymbolKind::Protocol, "objc(pl)");
 }
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1493,12 +1493,16 @@
 
 class IndexRequestCollector : public SymbolIndex {
 public:
+  IndexRequestCollector(std::vector Syms = {}) : Symbols(Syms) {}
+
   bool
   fuzzyFind(const FuzzyFindRequest &Req,
 llvm::function_ref Callback) const override {
 std::unique_lock Lock(Mut);
 Requests.push_back(Req);
 ReceivedRequestCV.notify_one();
+for (const auto &Sym : Symbols)
+  Callback(Sym);
 return true;
   }
 
@@ -1533,6 +1537,7 @@
   }
 
 private:
+  std::vector Symbols;
   // We need a mutex to handle async fuzzy find requests.
   mutable std::condition_variable ReceivedRequestCV;
   mutable std::mutex Mut;
@@ -3208,14 +3213,76 @@
   Symbol FoodClass = objcClass("FoodClass");
   Symbol SymFood = objcProtocol("Food");
   Symbol SymFooey = objcProtocol("Fooey");
-  auto Results = completions(R"objc(
-  id
-)objc",
+  auto Results = completions("id",
  {SymFood, FoodClass, SymFooey},
  /*Opts=*/{}, "Foo.m");
 
-  auto C = Results.Completions;
-  EXPECT_THAT(C, UnorderedElementsAre(named("Food"), named("Fooey")));
+  // Should only give protocols for ObjC protocol completions.
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(
+  AllOf(named("Food"), kind(CompletionItemKind::Interface)),
+  AllOf(named("Fooey"), kind(CompletionItemKind::Interface;
+
+  Results = completions("Fo^",
+{SymFood, FoodClass, SymFooey},
+/*Opts=*/{}, "Foo.m");
+  // Shouldn't give protocols for non protocol completions.
+  EXPECT_THAT(
+  Results.Completions,
+  ElementsAre(AllOf(named("FoodClass"), kind(CompletionItemKind::Class;
+}
+
+TEST(CompletionTest, ObjectiveCProtocolFromIndexSpeculation) {
+  MockFS FS;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
+
+  auto File = testPath("Foo.m");
+  Annotations Test(R"cpp(
+  @protocol Food
+  @end
+  id foo;
+  Foo$2^ bar;
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  Symbol FoodClass = objcClass("FoodClass");
+  IndexRequestCollector Requests({FoodClass});
+  Opts.Index = &Requests;
+
+  auto CompleteAtPoint = [&](StringRef P) {
+return cantFail(runCodeComplete(Server, File, Test.point(P), Opts))
+.Completions;
+  };
+
+  auto C = CompleteAtPoint("1");
+  auto Reqs1 = Requests.consumeRequests(1);
+  ASSERT_EQ(Reqs1.size(), 1u);
+  EXPECT_THAT(C, ElementsAre(AllOf(named("Food"),
+   kind(CompletionItemKind::Interface;
+
+  C = CompleteAtPoint("2");
+  auto Reqs2 = Requests.consumeRequests(1);
+  // Speculation succeeded. Used speculative index result, but filtering now to
+  // now include FoodClass.
+  ASSERT_EQ(Reqs2.size(), 1u);
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+  EXPECT_THAT(C, ElementsAre(AllOf(named("FoodClass"),
+   kind(CompletionItemKind::Class;
+}
+
+TEST(CompletionTest, Objective

[PATCH] D132962: [clangd][ObjC] Improve completions for protocols + category names

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 457258.
dgoldman added a comment.

Run clang-format again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132962

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/TestIndex.cpp
  clang-tools-extra/clangd/unittests/TestIndex.h

Index: clang-tools-extra/clangd/unittests/TestIndex.h
===
--- clang-tools-extra/clangd/unittests/TestIndex.h
+++ clang-tools-extra/clangd/unittests/TestIndex.h
@@ -39,6 +39,8 @@
llvm::StringRef USRPrefix);
 // Create an @interface or @implementation.
 Symbol objcClass(llvm::StringRef Name);
+// Create an @interface or @implementation category.
+Symbol objcCategory(llvm::StringRef Name, llvm::StringRef CategoryName);
 // Create an @protocol.
 Symbol objcProtocol(llvm::StringRef Name);
 
Index: clang-tools-extra/clangd/unittests/TestIndex.cpp
===
--- clang-tools-extra/clangd/unittests/TestIndex.cpp
+++ clang-tools-extra/clangd/unittests/TestIndex.cpp
@@ -99,6 +99,11 @@
   return objcSym(Name, index::SymbolKind::Class, "objc(cs)");
 }
 
+Symbol objcCategory(llvm::StringRef Name, llvm::StringRef CategoryName) {
+  std::string USRPrefix = ("objc(cy)" + Name + "@").str();
+  return objcSym(CategoryName, index::SymbolKind::Extension, USRPrefix);
+}
+
 Symbol objcProtocol(llvm::StringRef Name) {
   return objcSym(Name, index::SymbolKind::Protocol, "objc(pl)");
 }
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1493,12 +1493,16 @@
 
 class IndexRequestCollector : public SymbolIndex {
 public:
+  IndexRequestCollector(std::vector Syms = {}) : Symbols(Syms) {}
+
   bool
   fuzzyFind(const FuzzyFindRequest &Req,
 llvm::function_ref Callback) const override {
 std::unique_lock Lock(Mut);
 Requests.push_back(Req);
 ReceivedRequestCV.notify_one();
+for (const auto &Sym : Symbols)
+  Callback(Sym);
 return true;
   }
 
@@ -1533,6 +1537,7 @@
   }
 
 private:
+  std::vector Symbols;
   // We need a mutex to handle async fuzzy find requests.
   mutable std::condition_variable ReceivedRequestCV;
   mutable std::mutex Mut;
@@ -3208,14 +3213,74 @@
   Symbol FoodClass = objcClass("FoodClass");
   Symbol SymFood = objcProtocol("Food");
   Symbol SymFooey = objcProtocol("Fooey");
+  auto Results = completions("id", {SymFood, FoodClass, SymFooey},
+ /*Opts=*/{}, "Foo.m");
+
+  // Should only give protocols for ObjC protocol completions.
+  EXPECT_THAT(Results.Completions,
+  UnorderedElementsAre(
+  AllOf(named("Food"), kind(CompletionItemKind::Interface)),
+  AllOf(named("Fooey"), kind(CompletionItemKind::Interface;
+
+  Results = completions("Fo^", {SymFood, FoodClass, SymFooey},
+/*Opts=*/{}, "Foo.m");
+  // Shouldn't give protocols for non protocol completions.
+  EXPECT_THAT(
+  Results.Completions,
+  ElementsAre(AllOf(named("FoodClass"), kind(CompletionItemKind::Class;
+}
+
+TEST(CompletionTest, ObjectiveCProtocolFromIndexSpeculation) {
+  MockFS FS;
+  MockCompilationDatabase CDB;
+  ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
+
+  auto File = testPath("Foo.m");
+  Annotations Test(R"cpp(
+  @protocol Food
+  @end
+  id foo;
+  Foo$2^ bar;
+  )cpp");
+  runAddDocument(Server, File, Test.code());
+  clangd::CodeCompleteOptions Opts = {};
+
+  Symbol FoodClass = objcClass("FoodClass");
+  IndexRequestCollector Requests({FoodClass});
+  Opts.Index = &Requests;
+
+  auto CompleteAtPoint = [&](StringRef P) {
+return cantFail(runCodeComplete(Server, File, Test.point(P), Opts))
+.Completions;
+  };
+
+  auto C = CompleteAtPoint("1");
+  auto Reqs1 = Requests.consumeRequests(1);
+  ASSERT_EQ(Reqs1.size(), 1u);
+  EXPECT_THAT(C, ElementsAre(AllOf(named("Food"),
+   kind(CompletionItemKind::Interface;
+
+  C = CompleteAtPoint("2");
+  auto Reqs2 = Requests.consumeRequests(1);
+  // Speculation succeeded. Used speculative index result, but filtering now to
+  // now include FoodClass.
+  ASSERT_EQ(Reqs2.size(), 1u);
+  EXPECT_EQ(Reqs2[0], Reqs1[0]);
+  EXPECT_THAT(C, ElementsAre(AllOf(named("FoodClass"),
+   kind(CompletionItemKind::Class;
+}
+
+TEST(CompletionTest, ObjectiveCCategoryFromIndexIgnored) {
+  Symbol FoodCategory = objcCategory("FoodClass", "Extension");
   auto Results = completions(R"objc(
-  id
+  @interface Foo
+  @end
+  @interface Foo (^)
+  @end
   

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added reviewers: psmith, easyaspi314.
DavidSpickett added a comment.

The remaining reference in the repo is a #define in 
`openmp/runtime/src/kmp_platform.h` which seems harmless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133109

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

[PATCH] D132689: [Object] Refactor code for extracting offload binaries

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132689

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

[clang] 8dd14c4 - [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

[Wei Yi Tee via cfe-commits]

Author: Wei Yi Tee
Date: 2022-09-01T14:09:43Z
New Revision: 8dd14c427ae5c86cc2af4b8cac05b41bc4a49e86

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

LOG: [clang][dataflow] Use `StringMap` for storing analysis states at annotated 
points instead of `vector>`.

Reviewed By: gribozavr2, sgatev, ymandel

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

Added: 


Modified: 
clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 




diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
index c0c05b0efdbb..dbb06c85814f 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
@@ -19,6 +19,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include 
@@ -72,6 +73,7 @@ llvm::Expected>
 test::buildStatementToAnnotationMapping(const FunctionDecl *Func,
 llvm::Annotations AnnotatedCode) {
   llvm::DenseMap Result;
+  llvm::StringSet<> ExistingAnnotations;
 
   auto StmtMatcher =
   findAll(stmt(unless(anyOf(hasParent(expr()), hasParent(returnStmt()
@@ -113,7 +115,14 @@ test::buildStatementToAnnotationMapping(const FunctionDecl 
*Func,
 .data());
   }
 
-  Result[Stmt] = Code.slice(Range.Begin, Range.End).str();
+  auto Annotation = Code.slice(Range.Begin, Range.End).str();
+  if (!ExistingAnnotations.insert(Annotation).second) {
+return llvm::createStringError(
+std::make_error_code(std::errc::invalid_argument),
+"Repeated use of annotation: %s", Annotation.data());
+  }
+  Result[Stmt] = std::move(Annotation);
+
   I++;
 
   if (I < Annotations.size() && Annotations[I].Begin >= Offset) {

diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index 285c0c991d23..e3b68ca2976d 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -37,6 +37,7 @@
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
@@ -284,13 +285,12 @@ checkDataflow(AnalysisInputs AI,
 ///
 ///   Annotations must not be repeated.
 template 
-llvm::Error checkDataflow(
-AnalysisInputs AI,
-std::function>>,
-const AnalysisOutputs &)>
-VerifyResults) {
+llvm::Error
+checkDataflow(AnalysisInputs AI,
+  std::function> &,
+ const AnalysisOutputs &)>
+  VerifyResults) {
   // Compute mapping from nodes of annotated statements to the content in the
   // annotation.
   llvm::DenseMap StmtToAnnotations;
@@ -308,11 +308,9 @@ llvm::Error checkDataflow(
 
   using StateT = DataflowAnalysisState;
 
-  // FIXME: Use a string map instead of a vector of pairs.
-  //
   // Save the states computed for program points immediately following 
annotated
   // statements. The saved states are keyed by the content of the annotation.
-  std::vector> AnnotationStates;
+  llvm::StringMap AnnotationStates;
   auto PostVisitCFG = [&StmtToAnnotations, &AnnotationStates,
PrevPostVisitCFG = std::move(AI.PostVisitCFG)](
   ASTContext &Ctx, const CFGElement &Elt,
@@ -329,7 +327,10 @@ llvm::Error checkDataflow(
   return;
 auto *Lattice =
 llvm::any_cast(&State.Lattice.Value);
-AnnotationStates.emplace_back(It->second, StateT{*Lattice, State.Env});
+auto [_, InsertSuccess] =
+AnnotationStates.insert({It->second, StateT{*Lattice, State.Env}});
+(void)InsertSuccess;
+assert(InsertSuccess);
   };
   return checkDataflow(
   std::move(AI)
@@ -378,12 +379,20 @@ llvm::Error checkDataflow(
 std::move(MakeAnalysis))
   .withASTBuildArgs(std::move(Args))
   .withASTBuildVirtualMappedFiles(std::move(VirtualMappedFiles)),
-  [&VerifyResults](
-  llvm::ArrayRef>>
-  AnnotationStates,
-  const AnalysisOutputs &AO) {
-VerifyResults(AnnotationStates, AO.ASTCtx);
+  [&VerifyResults](const llvm::StringMap> &AnnotationStates,
+

[PATCH] D132763: [clang][dataflow] Use `StringMap` for storing analysis states at annotated points instead of `vector>`.

[weiyi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8dd14c427ae5: [clang][dataflow] Use `StringMap` for storing 
analysis states at annotated… (authored by wyt).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132763

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -426,12 +426,12 @@
std::pair>>
Results,
ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[3].second.Env;
-const Environment &Env2 = Results[2].second.Env;
-const Environment &Env3 = Results[1].second.Env;
-const Environment &Env4 = Results[0].second.Env;
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+ Pair("p3", _), Pair("p4", _)));
+const Environment &Env1 = Results[0].second.Env;
+const Environment &Env2 = Results[1].second.Env;
+const Environment &Env3 = Results[2].second.Env;
+const Environment &Env4 = Results[3].second.Env;
 
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
@@ -579,10 +579,10 @@
  Results,
  ASTContext &ASTCtx) {
 ASSERT_THAT(Results,
-ElementsAre(Pair("p3", _), Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[2].second.Env;
+ElementsAre(Pair("p1", _), Pair("p2", _), Pair("p3", _)));
+const Environment &Env1 = Results[0].second.Env;
 const Environment &Env2 = Results[1].second.Env;
-const Environment &Env3 = Results[0].second.Env;
+const Environment &Env3 = Results[2].second.Env;
 
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
@@ -622,12 +622,12 @@
  std::pair>>
  Results,
  ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[3].second.Env;
-const Environment &Env2 = Results[2].second.Env;
-const Environment &Env3 = Results[1].second.Env;
-const Environment &Env4 = Results[0].second.Env;
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _),
+ Pair("p3", _), Pair("p4", _)));
+const Environment &Env1 = Results[0].second.Env;
+const Environment &Env2 = Results[1].second.Env;
+const Environment &Env3 = Results[2].second.Env;
+const Environment &Env4 = Results[3].second.Env;
 
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
@@ -751,14 +751,14 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-const Environment &Env1 = Results[1].second.Env;
+const Environment &Env1 = Results[0].second.Env;
 auto *FooVal1 =
 cast(Env1.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env1.flowConditionImplies(*FooVal1));
 
-const Environment &Env2 = Results[0].second.Env;
+const Environment &Env2 = Results[1].second.Env;
 auto *FooVal2 =
 cast(Env2.getValue(*FooDecl, SkipPast::None));
 EXPECT_FALSE(Env2.flowConditionImplies(*FooVal2));
@@ -785,14 +785,14 @@
 const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
 ASSERT_THAT(FooDecl, NotNull());
 
-ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+ASSERT_THAT(Results, ElementsAre(Pair("p1", _), Pair("p2", _)));
 
-const Environment &Env1 = Results[1].second.Env;
+const Env

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

[Luke Nihlen via Phabricator via cfe-commits]

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

Refactor to use CodeSynthesisContexts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133052

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -256,3 +256,20 @@
 C auto **&j2 = g();  // expected-error {{deduced type 'int' does not satisfy 
'C'}}
 C auto **&&j3 = g(); // expected-error {{deduced type 'int' does not satisfy 
'C'}}
 }
+
+namespace Issue50891 {
+
+template 
+concept Numeric =
+requires(T a) {
+foo(a);
+};
+
+struct Deferred {
+friend void foo(Deferred);
+template  operator TO();
+};
+
+static_assert(Numeric); // should not crash clang
+
+} // namespace Issue50891
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -4009,6 +4009,16 @@
 }
   }
 
+  if (!S.CodeSynthesisContexts.empty())
+llvm::errs() << "Kind: " << S.CodeSynthesisContexts.back().Kind << "\n";
+
+  // Avoid an infinite template expansion loop in requirements checking by
+  // skipping the conversion functions check.
+  bool InRequirementInstantiation =
+  !S.CodeSynthesisContexts.empty() &&
+  S.CodeSynthesisContexts.back().Kind ==
+  Sema::CodeSynthesisContext::RequirementInstantiation;
+
   // FIXME: Work around a bug in C++17 guaranteed copy elision.
   //
   // When initializing an object of class type T by constructor
@@ -4021,7 +4031,7 @@
   // Note: SecondStepOfCopyInit is only ever true in this case when
   // evaluating whether to produce a C++98 compatibility warning.
   if (S.getLangOpts().CPlusPlus17 && Args.size() == 1 &&
-  !SecondStepOfCopyInit) {
+  !SecondStepOfCopyInit && !InRequirementInstantiation) {
 Expr *Initializer = Args[0];
 auto *SourceRD = Initializer->getType()->getAsCXXRecordDecl();
 if (SourceRD && S.isCompleteType(DeclLoc, Initializer->getType())) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -210,7 +210,8 @@
   This fixes `GH51182 `
 - Fixes an assert crash caused by looking up missing vtable information on 
``consteval``
   virtual functions. Fixes `GH55065 
`_.
-
+- Fixed a crash during template instantiation on a conversion operator during 
constraint
+  evaulation. Fixes `GH50891 
`_.
 
 C++2b Feature Support
 ^


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -256,3 +256,20 @@
 C auto **&j2 = g();  // expected-error {{deduced type 'int' does not satisfy 'C'}}
 C auto **&&j3 = g(); // expected-error {{deduced type 'int' does not satisfy 'C'}}
 }
+
+namespace Issue50891 {
+
+template 
+concept Numeric =
+requires(T a) {
+foo(a);
+};
+
+struct Deferred {
+friend void foo(Deferred);
+template  operator TO();
+};
+
+static_assert(Numeric); // should not crash clang
+
+} // namespace Issue50891
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -4009,6 +4009,16 @@
 }
   }
 
+  if (!S.CodeSynthesisContexts.empty())
+llvm::errs() << "Kind: " << S.CodeSynthesisContexts.back().Kind << "\n";
+
+  // Avoid an infinite template expansion loop in requirements checking by
+  // skipping the conversion functions check.
+  bool InRequirementInstantiation =
+  !S.CodeSynthesisContexts.empty() &&
+  S.CodeSynthesisContexts.back().Kind ==
+  Sema::CodeSynthesisContext::RequirementInstantiation;
+
   // FIXME: Work around a bug in C++17 guaranteed copy elision.
   //
   // When initializing an object of class type T by constructor
@@ -4021,7 +4031,7 @@
   // Note: SecondStepOfCopyInit is only ever true in this case when
   // evaluating whether to produce a C++98 compatibility warning.
   if (S.getLangOpts().CPlusPlus17 && Args.size() == 1 &&
-  !SecondStepOfCopyInit) {
+  !SecondStepOfCopyInit && !InRequirementInstantiation) {
 Expr *Initializer = Args[0];
 auto *SourceRD = Initializer->getType()->getAsCXXRecordDecl();
 if (SourceRD && S.isCompleteType(DeclLoc, Initializer->getType())) {
Index: clang/docs/ReleaseNotes.rst
===
--

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

[Luke Nihlen via Phabricator via cfe-commits]

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

Remove debugging cruft


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133052

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -256,3 +256,20 @@
 C auto **&j2 = g();  // expected-error {{deduced type 'int' does not satisfy 
'C'}}
 C auto **&&j3 = g(); // expected-error {{deduced type 'int' does not satisfy 
'C'}}
 }
+
+namespace Issue50891 {
+
+template 
+concept Numeric =
+requires(T a) {
+foo(a);
+};
+
+struct Deferred {
+friend void foo(Deferred);
+template  operator TO();
+};
+
+static_assert(Numeric); // should not crash clang
+
+} // namespace Issue50891
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -4009,6 +4009,13 @@
 }
   }
 
+  // Avoid an infinite template expansion loop in requirements checking by
+  // skipping the conversion functions check.
+  bool InRequirementInstantiation =
+  !S.CodeSynthesisContexts.empty() &&
+  S.CodeSynthesisContexts.back().Kind ==
+  Sema::CodeSynthesisContext::RequirementInstantiation;
+
   // FIXME: Work around a bug in C++17 guaranteed copy elision.
   //
   // When initializing an object of class type T by constructor
@@ -4021,7 +4028,7 @@
   // Note: SecondStepOfCopyInit is only ever true in this case when
   // evaluating whether to produce a C++98 compatibility warning.
   if (S.getLangOpts().CPlusPlus17 && Args.size() == 1 &&
-  !SecondStepOfCopyInit) {
+  !SecondStepOfCopyInit && !InRequirementInstantiation) {
 Expr *Initializer = Args[0];
 auto *SourceRD = Initializer->getType()->getAsCXXRecordDecl();
 if (SourceRD && S.isCompleteType(DeclLoc, Initializer->getType())) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -210,7 +210,8 @@
   This fixes `GH51182 `
 - Fixes an assert crash caused by looking up missing vtable information on 
``consteval``
   virtual functions. Fixes `GH55065 
`_.
-
+- Fixed a crash during template instantiation on a conversion operator during 
constraint
+  evaulation. Fixes `GH50891 
`_.
 
 C++2b Feature Support
 ^


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -256,3 +256,20 @@
 C auto **&j2 = g();  // expected-error {{deduced type 'int' does not satisfy 'C'}}
 C auto **&&j3 = g(); // expected-error {{deduced type 'int' does not satisfy 'C'}}
 }
+
+namespace Issue50891 {
+
+template 
+concept Numeric =
+requires(T a) {
+foo(a);
+};
+
+struct Deferred {
+friend void foo(Deferred);
+template  operator TO();
+};
+
+static_assert(Numeric); // should not crash clang
+
+} // namespace Issue50891
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -4009,6 +4009,13 @@
 }
   }
 
+  // Avoid an infinite template expansion loop in requirements checking by
+  // skipping the conversion functions check.
+  bool InRequirementInstantiation =
+  !S.CodeSynthesisContexts.empty() &&
+  S.CodeSynthesisContexts.back().Kind ==
+  Sema::CodeSynthesisContext::RequirementInstantiation;
+
   // FIXME: Work around a bug in C++17 guaranteed copy elision.
   //
   // When initializing an object of class type T by constructor
@@ -4021,7 +4028,7 @@
   // Note: SecondStepOfCopyInit is only ever true in this case when
   // evaluating whether to produce a C++98 compatibility warning.
   if (S.getLangOpts().CPlusPlus17 && Args.size() == 1 &&
-  !SecondStepOfCopyInit) {
+  !SecondStepOfCopyInit && !InRequirementInstantiation) {
 Expr *Initializer = Args[0];
 auto *SourceRD = Initializer->getType()->getAsCXXRecordDecl();
 if (SourceRD && S.isCompleteType(DeclLoc, Initializer->getType())) {
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -210,7 +210,8 @@
   This fixes `GH51182 `
 - Fixes an assert crash caused by looking up missing vtable information on ``consteval``
   virtua

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

[Luke Nihlen via Phabricator via cfe-commits]

luken-google added a comment.

In D133052#3763201 , @ychen wrote:

> In D133052#3763041 , @luken-google 
> wrote:
>
>> In D133052#3762596 , @ychen wrote:
>>
>>> Like mentioned in 
>>> https://stackoverflow.com/questions/68853472/is-this-a-bug-in-clangs-c20-concepts-implementation-unnecessary-checking-of,
>>>  could we not go down the path of considering conversion candidates? It 
>>> seems that's blessed by the standard.
>>
>> If I'm understanding the code correctly, the intent of this patch is to 
>> definitely consider conversion candidates, only to exclude those conversion 
>> candidates that are templated methods where the From type is the same as the 
>> To type, which to me mean they are possibly redundant?
>
> Excluding them is basically saying "because it may be a redundant conversion, 
> we should not consider it as the best via function." which doesn't seem 
> correct to me.
>
> I think the straightforward approach would be to check if we're in the 
> `ConstraintCheck` instantiation context, and if so check if any template 
> parameter is constrained by the same concept. However, I'm worried about the 
> overhead. So I'd prefer to skip this add-conv-candicates-for-copy-elision 
> path 
> (https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaInit.cpp#L4012-L4053)
>  during the concept check. The compiler guarantees copy elision under certain 
> conditions (C++17) but I could not think of any situation that users want to 
> or could check copy elision inside the concept. So I think we're safe.

Thanks for your suggestion, I didn't know about the context member in Sema. I 
agree I think this is a much better approach than my original. While looping 
the code is in the `RequirementInstantiation` context, so that's the one I've 
keyed off here. Please let me know if this is what you had in mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133052

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

[PATCH] D133085: [clang] trim trailing space in format tests. NFC

[YingChi Long via Phabricator via cfe-commits]

inclyc added a comment.

Thank you for your patience and detailed explanation! Sorry for waste your time 
though (


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133085

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5425
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
-  if (CCGenDiagnostics && A) {
-SmallString<128> CrashDirectory(A->getValue());
+  const char *CrashDirectory = CCGenDiagnostics && A
+   ? A->getValue()

erichkeane wrote:
> `StringRef` would be better here instead, which should mean you don't have to 
> create the SmallString below, and could just work in `Twine`s for everything.
It seems that `llvm::sys::path::append` takes twine as inputs, but it only 
outputs to a SmallVectorImpl.
Unless there is something else I could use?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5425
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
-  if (CCGenDiagnostics && A) {
-SmallString<128> CrashDirectory(A->getValue());
+  const char *CrashDirectory = CCGenDiagnostics && A
+   ? A->getValue()

mizvekov wrote:
> erichkeane wrote:
> > `StringRef` would be better here instead, which should mean you don't have 
> > to create the SmallString below, and could just work in `Twine`s for 
> > everything.
> It seems that `llvm::sys::path::append` takes twine as inputs, but it only 
> outputs to a SmallVectorImpl.
> Unless there is something else I could use?
Ah, yikes, I  missed that was what was happening :/  THAT is still necessary.  
A Twine can be created from a stringref (and I'd still prefer one anyway), but 
you DO still have to make that SmallString unfortunately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133085: [clang] trim trailing space in format tests. NFC

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133085#3764067 , @inclyc wrote:

> Thank you for your patience and detailed explanation! Sorry for waste your 
> time though (

No worries, it's definitely not a waste of time to explain this sort of stuff! 
:-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133085

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

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

[Peter Smith via Phabricator via cfe-commits]

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

LGTM. GCC no longer supports Arm architecture prior to v4 so it is likely the 
alternative of adding support for v3 is not worth it. The only Arm machines 
running v2 are likely to be the Acorn Archimedes family, these have their own 
object file format so are unlikely to benefit from LLVM support.

Agree that the openmp reference looks harmless.

Will be worth waiting to see if Nick has any objections.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133109

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5425
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
-  if (CCGenDiagnostics && A) {
-SmallString<128> CrashDirectory(A->getValue());
+  const char *CrashDirectory = CCGenDiagnostics && A
+   ? A->getValue()

erichkeane wrote:
> mizvekov wrote:
> > erichkeane wrote:
> > > `StringRef` would be better here instead, which should mean you don't 
> > > have to create the SmallString below, and could just work in `Twine`s for 
> > > everything.
> > It seems that `llvm::sys::path::append` takes twine as inputs, but it only 
> > outputs to a SmallVectorImpl.
> > Unless there is something else I could use?
> Ah, yikes, I  missed that was what was happening :/  THAT is still necessary. 
>  A Twine can be created from a stringref (and I'd still prefer one anyway), 
> but you DO still have to make that SmallString unfortunately.
The Twine also can't take a nullptr (such as the case no flag passed and no env 
variable), so we would have to use something like `std::optional`, and 
make the needed conversions here.

And the Twine to SmallString copy looks unergonomic as well.

So it seems we would need to make some little changes out of scope of this MR 
to get that to look nicely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5425
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
-  if (CCGenDiagnostics && A) {
-SmallString<128> CrashDirectory(A->getValue());
+  const char *CrashDirectory = CCGenDiagnostics && A
+   ? A->getValue()

mizvekov wrote:
> erichkeane wrote:
> > mizvekov wrote:
> > > erichkeane wrote:
> > > > `StringRef` would be better here instead, which should mean you don't 
> > > > have to create the SmallString below, and could just work in `Twine`s 
> > > > for everything.
> > > It seems that `llvm::sys::path::append` takes twine as inputs, but it 
> > > only outputs to a SmallVectorImpl.
> > > Unless there is something else I could use?
> > Ah, yikes, I  missed that was what was happening :/  THAT is still 
> > necessary.  A Twine can be created from a stringref (and I'd still prefer 
> > one anyway), but you DO still have to make that SmallString unfortunately.
> The Twine also can't take a nullptr (such as the case no flag passed and no 
> env variable), so we would have to use something like `std::optional`, 
> and make the needed conversions here.
> 
> And the Twine to SmallString copy looks unergonomic as well.
> 
> So it seems we would need to make some little changes out of scope of this MR 
> to get that to look nicely.
I guess i don't get the concern here?  I'm suggesting just changing this from 
`const char*` to `StringRef` (which should work).  Everything else below should 
'just work' AFAIK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5425
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
-  if (CCGenDiagnostics && A) {
-SmallString<128> CrashDirectory(A->getValue());
+  const char *CrashDirectory = CCGenDiagnostics && A
+   ? A->getValue()

erichkeane wrote:
> mizvekov wrote:
> > erichkeane wrote:
> > > mizvekov wrote:
> > > > erichkeane wrote:
> > > > > `StringRef` would be better here instead, which should mean you don't 
> > > > > have to create the SmallString below, and could just work in `Twine`s 
> > > > > for everything.
> > > > It seems that `llvm::sys::path::append` takes twine as inputs, but it 
> > > > only outputs to a SmallVectorImpl.
> > > > Unless there is something else I could use?
> > > Ah, yikes, I  missed that was what was happening :/  THAT is still 
> > > necessary.  A Twine can be created from a stringref (and I'd still prefer 
> > > one anyway), but you DO still have to make that SmallString unfortunately.
> > The Twine also can't take a nullptr (such as the case no flag passed and no 
> > env variable), so we would have to use something like 
> > `std::optional`, and make the needed conversions here.
> > 
> > And the Twine to SmallString copy looks unergonomic as well.
> > 
> > So it seems we would need to make some little changes out of scope of this 
> > MR to get that to look nicely.
> I guess i don't get the concern here?  I'm suggesting just changing this from 
> `const char*` to `StringRef` (which should work).  Everything else below 
> should 'just work' AFAIK.
Oh okay I misunderstood you! Thought you meant to make CrashDirectory a Twine.

Yeah with `StringRef` we only need to change the test:

```
if (CrashDirectory) {
```
--->
```
if (CrashDirectory.data() != nullptr) {
```

Everything else stays the same. It doesn't look cleaner, but would address your 
concerns about avoiding storing a char pointer variable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5425
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
-  if (CCGenDiagnostics && A) {
-SmallString<128> CrashDirectory(A->getValue());
+  const char *CrashDirectory = CCGenDiagnostics && A
+   ? A->getValue()

mizvekov wrote:
> erichkeane wrote:
> > mizvekov wrote:
> > > erichkeane wrote:
> > > > mizvekov wrote:
> > > > > erichkeane wrote:
> > > > > > `StringRef` would be better here instead, which should mean you 
> > > > > > don't have to create the SmallString below, and could just work in 
> > > > > > `Twine`s for everything.
> > > > > It seems that `llvm::sys::path::append` takes twine as inputs, but it 
> > > > > only outputs to a SmallVectorImpl.
> > > > > Unless there is something else I could use?
> > > > Ah, yikes, I  missed that was what was happening :/  THAT is still 
> > > > necessary.  A Twine can be created from a stringref (and I'd still 
> > > > prefer one anyway), but you DO still have to make that SmallString 
> > > > unfortunately.
> > > The Twine also can't take a nullptr (such as the case no flag passed and 
> > > no env variable), so we would have to use something like 
> > > `std::optional`, and make the needed conversions here.
> > > 
> > > And the Twine to SmallString copy looks unergonomic as well.
> > > 
> > > So it seems we would need to make some little changes out of scope of 
> > > this MR to get that to look nicely.
> > I guess i don't get the concern here?  I'm suggesting just changing this 
> > from `const char*` to `StringRef` (which should work).  Everything else 
> > below should 'just work' AFAIK.
> Oh okay I misunderstood you! Thought you meant to make CrashDirectory a Twine.
> 
> Yeah with `StringRef` we only need to change the test:
> 
> ```
> if (CrashDirectory) {
> ```
> --->
> ```
> if (CrashDirectory.data() != nullptr) {
> ```
> 
> Everything else stays the same. It doesn't look cleaner, but would address 
> your concerns about avoiding storing a char pointer variable?
We could add a bool conversion to StringRef, but perhaps folks could find the 
meaning confusing between empty and null pointer?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D132550: Changes to code ownership in clang and clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 457289.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.
Herald added subscribers: sstefan1, arphaman.
Herald added a reviewer: jdoerfert.

Because I'm touching this file so much anyway to add the GitHub handles, I had 
some ideas I wanted to float to see if others thought they were worthwhile:

1. I'd like to rearrange the file so that we don't organize it by surname 
(slightly problematic given that not everyone has a surname or goes by their 
given name anyway) but instead organize it by component. I think most of the 
people who are going to be interested in the contents of this file are going to 
be looking for a component to find a name instead of looking for a name to find 
a component.
2. I'd like to convert the file to RST format. This not only gives us some 
nicer ways to categorize the file contents, but it also makes it easier to 
publish this file on the website along with our other developer policy 
documentation.
3. It gives us a nice way to organize former code owners without making it more 
difficult to find active code owners (I did not try to find all former code 
owners)

This latest patch shows you what I have in mind for the changes. I'm not 
strongly tied to making the file into RST as that does reduce the readability 
of the text file somewhat, but I think the benefits to making this 
documentation more public outweigh the downsides to using markup.

(If someone knows of a way to get RST to style headings that are cut off the 
local table of contents due to the `:depth:` annotation, that would be lovely. 
Right now, the third level headings are as visually distinct as I'd like 
because the local table of contents is limited to two levels of nesting.)


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

https://reviews.llvm.org/D132550

Files:
  clang-tools-extra/CODE_OWNERS.TXT
  clang/CODE_OWNERS.TXT
  clang/CodeOwners.rst
  clang/docs/CodeOwners.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -98,6 +98,7 @@
 .. toctree::
:maxdepth: 1
 
+   CodeOwners
InternalsManual
DriverInternals
OffloadingDesign
Index: clang/docs/CodeOwners.rst
===
--- /dev/null
+++ clang/docs/CodeOwners.rst
@@ -0,0 +1 @@
+.. include:: ../CodeOwners.rst
Index: clang/CodeOwners.rst
===
--- /dev/null
+++ clang/CodeOwners.rst
@@ -0,0 +1,256 @@
+=
+Clang Code Owners
+=
+
+This file is a list of the people responsible for ensuring that patches for a
+particular part of Clang are reviewed, either by themself or by someone else.
+They are also the gatekeepers for their part of Clang, with the final word on
+what goes in or not.
+
+.. contents::
+   :depth: 2
+   :local:
+
+Current Code Owners
+===
+The following people are the active code owners for the project. Please reach
+out to them for code reviews, questions about their area of expertise, or other
+assistance.
+
+All parts of Clang not covered by someone else
+--
+| Aaron Ballman
+| aaron\@aaronballman.com (email), aaron.ballman (Phabricator), AaronBallman (GitHub)
+
+
+Contained Components
+
+These code owners are responsible for particular high-level components within
+Clang that are typically contained to one area of the compiler.
+
+AST matchers
+
+| Manuel Klimek
+| klimek\@google.com (email), klimek (Phabricator), r4nt (GitHub)
+
+
+Clang LLVM IR generation
+
+| John McCall
+| rjmccall\@apple.com (email), rjmccall (Phabricator), rjmccall (GitHub)
+
+| Eli Friedman
+| efriedma\@quicinc.com (email), efriedma (Phabricator), efriedma-quic (GitHub)
+
+| Anton Korobeynikov
+| anton\@korobeynikov.info (email), asl (Phabricator), asl (GitHub)
+
+
+Analysis & CFG
+~~
+| Dmitri Gribenko
+| gribozavr\@gmail.com (email), gribozavr (Phabricator), gribozavr (GitHub)
+
+| Yitzhak Mandelbaum
+| yitzhakm\@google.com (email), ymandel (Phabricator), ymand (GitHub)
+
+| Stanislav Gatev
+| sgatev\@google.com (email), sgatev (Phabricator), sgatev (GitHub)
+
+
+Modules & serialization
+~~~
+| Chuanqi Xu
+| yedeng.yd\@linux.alibaba.com (email), ChuanqiXu (Phabricator), ChuanqiXu9 (GitHub)
+
+| Michael Spencer
+| bigcheesegs\@gmail.com (email), Bigcheese (Phabricator), Bigcheese (GitHub)
+
+
+Templates
+~
+| Erich Keane
+| erich.keane\@intel.com (email), ErichKeane (Phabricator), erichkeane (GitHub)
+
+
+Debug information
+~
+| Eric Christopher
+| echristo\@gmail.com (email), echristo (Phabricator), echristo (GitHub)
+
+
+Exception handling
+~~
+| Anton Korobeynikov
+| anton\@korobeynikov.info (email), asl (

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

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

balazske created this revision.
Herald added subscribers: carlosgalvezp, steakhal, martong, gamesh411, 
Szelethus, dkrupp, xazax.hun, mgorny.
Herald added a project: All.
balazske requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Add a check to detect usages of `realloc` where the result is assigned
to the same variable (or field) as passed to the first argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133119

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-realloc-usage %t
+
+void *realloc(void *, __SIZE_TYPE__);
+
+namespace std {
+  using ::realloc;
+}
+
+namespace n1 {
+  void *p;
+}
+
+namespace n2 {
+  void *p;
+}
+
+struct P {
+  void *p;
+  void *q;
+  P *pp;
+  void *&f();
+};
+
+struct P1 {
+  static void *p;
+  static void *q;
+};
+
+template
+struct P2 {
+  static void *p;
+  static void *q;
+};
+
+template
+void templ(void *p) {
+  A::p = realloc(A::p, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: memory block passed to 'realloc' may leak if 'realloc' fails [bugprone-suspicious-realloc-usage]
+  p = realloc(A::p, 10);
+  A::q = realloc(A::p, 10);
+  A::p = realloc(B::p, 10);
+  P2::p = realloc(P2::p, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: memory block passed to 'realloc' may leak if 'realloc' fails [bugprone-suspicious-realloc-usage]
+  P2::p = realloc(P2::p, 1);
+}
+
+void *&getPtr();
+P &getP();
+
+void foo(void *p, P *p1, int *pi) {
+  p = realloc(p, 111);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: memory block passed to 'realloc' may leak if 'realloc' fails [bugprone-suspicious-realloc-usage]
+
+  p = std::realloc(p, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: memory block passed to 'realloc' may leak if 'realloc' fails [bugprone-suspicious-realloc-usage]
+
+  p1->p = realloc(p1->p, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: memory block passed to 'realloc' may leak if 'realloc' fails [bugprone-suspicious-realloc-usage]
+
+  p1->pp->p = realloc(p1->pp->p, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: memory block passed to 'realloc' may leak if 'realloc' fails [bugprone-suspicious-realloc-usage]
+
+  pi = (int*)realloc(pi, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: memory block passed to 'realloc' may leak if 'realloc' fails [bugprone-suspicious-realloc-usage]
+
+  templ>(p);
+}
+
+void no_warn(void *p, P *p1, P *p2) {
+  void *q = realloc(p, 10);
+  q = realloc(p, 10);
+  p1->q = realloc(p1->p, 10);
+  p2->p = realloc(p1->p, 20);
+  n1::p = realloc(n2::p, 30);
+  p1->pp->p = realloc(p1->p, 10);
+  getPtr() = realloc(getPtr(), 30);
+  getP().p = realloc(getP().p, 20);
+  p1->f() = realloc(p1->f(), 30);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -124,6 +124,7 @@
`bugprone-suspicious-memory-comparison `_,
`bugprone-suspicious-memset-usage `_, "Yes"
`bugprone-suspicious-missing-comma `_,
+   `bugprone-suspicious-realloc-usage `_,
`bugprone-suspicious-semicolon `_, "Yes"
`bugprone-suspicious-string-compare `_, "Yes"
`bugprone-swapped-arguments `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst
@@ -0,0 +1,31 @@
+.. title:: clang-tidy - bugprone-suspicious-realloc-usage
+
+bugprone-suspicious-realloc-usage
+=
+
+This check finds usages of ``realloc`` where the return value is assigned to the
+same expression as passed to the first argument:
+``p = realloc(p, size);``
+The problem with this construct is that if ``realloc`` fails it returns a
+null pointer but does not deallocate the original memory. The original
+memory block is not available any more for the program to use or free (unless
+another variable is pointing to it).
+
+The pointer expression can be a variable or a field member of a data

[clang] fb38bae - Removing an accidentally duplicated heading; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-09-01T11:44:11-04:00
New Revision: fb38baef8727fbdee0babbe74620f4f6755a9fda

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

LOG: Removing an accidentally duplicated heading; NFC

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index bb7c508001db4..8034e79860182 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -131,10 +131,6 @@ Improvements to Clang's diagnostics
 Non-comprehensive list of changes in this release
 -
 
-
-Non-comprehensive list of changes in this release
--
-
 New Compiler Flags
 --
 



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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5425
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
-  if (CCGenDiagnostics && A) {
-SmallString<128> CrashDirectory(A->getValue());
+  const char *CrashDirectory = CCGenDiagnostics && A
+   ? A->getValue()

mizvekov wrote:
> mizvekov wrote:
> > erichkeane wrote:
> > > mizvekov wrote:
> > > > erichkeane wrote:
> > > > > mizvekov wrote:
> > > > > > erichkeane wrote:
> > > > > > > `StringRef` would be better here instead, which should mean you 
> > > > > > > don't have to create the SmallString below, and could just work 
> > > > > > > in `Twine`s for everything.
> > > > > > It seems that `llvm::sys::path::append` takes twine as inputs, but 
> > > > > > it only outputs to a SmallVectorImpl.
> > > > > > Unless there is something else I could use?
> > > > > Ah, yikes, I  missed that was what was happening :/  THAT is still 
> > > > > necessary.  A Twine can be created from a stringref (and I'd still 
> > > > > prefer one anyway), but you DO still have to make that SmallString 
> > > > > unfortunately.
> > > > The Twine also can't take a nullptr (such as the case no flag passed 
> > > > and no env variable), so we would have to use something like 
> > > > `std::optional`, and make the needed conversions here.
> > > > 
> > > > And the Twine to SmallString copy looks unergonomic as well.
> > > > 
> > > > So it seems we would need to make some little changes out of scope of 
> > > > this MR to get that to look nicely.
> > > I guess i don't get the concern here?  I'm suggesting just changing this 
> > > from `const char*` to `StringRef` (which should work).  Everything else 
> > > below should 'just work' AFAIK.
> > Oh okay I misunderstood you! Thought you meant to make CrashDirectory a 
> > Twine.
> > 
> > Yeah with `StringRef` we only need to change the test:
> > 
> > ```
> > if (CrashDirectory) {
> > ```
> > --->
> > ```
> > if (CrashDirectory.data() != nullptr) {
> > ```
> > 
> > Everything else stays the same. It doesn't look cleaner, but would address 
> > your concerns about avoiding storing a char pointer variable?
> We could add a bool conversion to StringRef, but perhaps folks could find the 
> meaning confusing between empty and null pointer?
Hrm... do we actually have meaning here to 'empty' for these files?  I guess we 
allow it already hrm... Ok, disregard the suggestion, and thanks for 
thinking this through with me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[clang] 9599393 - Revert "[Pipelines] Introduce DAE after ArgumentPromotion"

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2022-09-01T08:52:19-07:00
New Revision: 9599393eebf7b5ef46a144938f593e812dd01a18

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

LOG: Revert "[Pipelines] Introduce DAE after ArgumentPromotion"

This reverts commit b10a341aa5b0b93b9175a8f11efc9a0955ab361e.

This commit exposes the pre-existing 
https://github.com/llvm/llvm-project/issues/56503 in some edge cases. Will fix 
that and then reland this.

Added: 


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

Removed: 




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

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

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

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

I've run into https://github.com/llvm/llvm-project/issues/56503 after this 
patch so I've reverted this for now while I'm fixing that. Sorry for this patch 
exposing so many pre-existing issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5425
   Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
-  if (CCGenDiagnostics && A) {
-SmallString<128> CrashDirectory(A->getValue());
+  const char *CrashDirectory = CCGenDiagnostics && A
+   ? A->getValue()

erichkeane wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > erichkeane wrote:
> > > > mizvekov wrote:
> > > > > erichkeane wrote:
> > > > > > mizvekov wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > `StringRef` would be better here instead, which should mean you 
> > > > > > > > don't have to create the SmallString below, and could just work 
> > > > > > > > in `Twine`s for everything.
> > > > > > > It seems that `llvm::sys::path::append` takes twine as inputs, 
> > > > > > > but it only outputs to a SmallVectorImpl.
> > > > > > > Unless there is something else I could use?
> > > > > > Ah, yikes, I  missed that was what was happening :/  THAT is still 
> > > > > > necessary.  A Twine can be created from a stringref (and I'd still 
> > > > > > prefer one anyway), but you DO still have to make that SmallString 
> > > > > > unfortunately.
> > > > > The Twine also can't take a nullptr (such as the case no flag passed 
> > > > > and no env variable), so we would have to use something like 
> > > > > `std::optional`, and make the needed conversions here.
> > > > > 
> > > > > And the Twine to SmallString copy looks unergonomic as well.
> > > > > 
> > > > > So it seems we would need to make some little changes out of scope of 
> > > > > this MR to get that to look nicely.
> > > > I guess i don't get the concern here?  I'm suggesting just changing 
> > > > this from `const char*` to `StringRef` (which should work).  Everything 
> > > > else below should 'just work' AFAIK.
> > > Oh okay I misunderstood you! Thought you meant to make CrashDirectory a 
> > > Twine.
> > > 
> > > Yeah with `StringRef` we only need to change the test:
> > > 
> > > ```
> > > if (CrashDirectory) {
> > > ```
> > > --->
> > > ```
> > > if (CrashDirectory.data() != nullptr) {
> > > ```
> > > 
> > > Everything else stays the same. It doesn't look cleaner, but would 
> > > address your concerns about avoiding storing a char pointer variable?
> > We could add a bool conversion to StringRef, but perhaps folks could find 
> > the meaning confusing between empty and null pointer?
> Hrm... do we actually have meaning here to 'empty' for these files?  I guess 
> we allow it already hrm... Ok, disregard the suggestion, and thanks for 
> thinking this through with me.
Thanks for the review and suggestions!

Yeah right, I didn't want to change the behavior vs empty here, but I think if 
we did some change, it would make more sense to create some specific error for 
it, instead of treating empty the same as no argument passed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: cor3ntin, tahonermann.
aaron.ballman added a comment.

In D133082#3763829 , @erichkeane 
wrote:

> I don't mind the idea, but wonder if some level of RFC/project-wide decision 
> should be made here?  WDYT @aaron.ballman ?

We use environment variables in several other places within the driver, but I 
am a bit skittish about adding new ones to correspond to feature flags as that 
seems to be somewhat novel. I think it'd be a good idea to get more buy-in with 
an RFC on Discourse for more broad awareness.

If we go this route, we definitely need user-facing documentation that explains 
what's going on. I don't think we have anything corresponding to 
https://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html, but having such 
a page for Clang would be a good idea (even if the page is incomplete).




Comment at: clang/docs/ReleaseNotes.rst:135-144
 Non-comprehensive list of changes in this release
 -
 
 New Compiler Flags
 --
+- It's now possible to set the crash diagnostics directory through
+  the environment variable 'CLANG_CRASH_DIAGNOSTICS_DIR'.





Comment at: clang/lib/Driver/Driver.cpp:5427
+   ? A->getValue()
+   : 
std::getenv("CLANG_CRASH_DIAGNOSTICS_DIR");
+  if (CrashDirectory) {

This should use `llvm::sys::Process::GetEnv()` so it properly handles text 
encodings. CC @tahonermann @cor3ntin for text encoding awareness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

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

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki updated this revision to Diff 457294.
yusuke-kadowaki added a comment.

- Update tests
- Fix document


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,167 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // Align these\n"
+   "\n"
+   "#include \"aa.h\" // two comments\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\"\n"
+   "#include \"aaa.h\" // But do not align this because there are two lines without comments above\n",
+   Style);
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234; // This is a very long\n"
+"  // comment which is\n"
+"  // wrapped arround.\n"
+"\n"
+"int x =\n"
+"2; // Is this still aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 40;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

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

steakhal added a comment.

Consider adding a test case demonstrating if the checker recognizes and 
suppresses the check if there are multiple variables referring to the memory 
being reallocated.
If the check does not handle that case, we should probably state this 
limitation explicitly. And marking the corresponding test case with a FIXME.

I was thinking about something like this:

  void escape(void *);
  void foo(void *p) {
void *q = p;
p = realloc(p, 10); // no-warning
escape(q);
  }
  void bar(void *p) {
p = realloc(p, 10); // warn: ...
void *q = p;
escape(q);
  }

The `MallocOverflowCheck` does something similar. It uses 
`isBeforeInTranslationUnit()`.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp:20-21
+namespace {
+class IsSamePtrExpr : public clang::StmtVisitor /*,
+ public clang::DeclVisitor*/
+{

Commented code?



Comment at: 
clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp:23
+{
+  /// The other expression to compare with.
+  /// This variable is used to pass the data from a \c check function to any of

against


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133119

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

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

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki marked an inline comment as done.
yusuke-kadowaki added inline comments.



Comment at: clang/include/clang/Format/Format.h:141
 
-  /// Alignment options.
-  ///
-  /// They can also be read as a whole for compatibility. The choices are:
+  /// Alignment styles of ``AlignConsecutiveStyle`` are:
   /// - None

HazardyKnusperkeks wrote:
> That I wouldn't change.
reverted



Comment at: clang/include/clang/Format/Format.h:154
   /// \code
   ///   AlignConsecutiveMacros: AcrossEmptyLines
   ///

HazardyKnusperkeks wrote:
> The change/addition has to be here, since here it directly states 
> `AlignConsecutiveMacros`.
What are you meaning to say here? I thought saying `AlignConsecutiveStyle` is 
used for multiple options is what we are trying to do.

Should we say something like, 
> Here AlignConsecutiveMacros is used as an example, but in practice 
> AlignConsecutiveStyle is also used with other options.
in this place?



Comment at: clang/unittests/Format/FormatTestComments.cpp:2863
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveTrailingComments.AcrossEmptyLines = true;
+  verifyFormat("#include \"a.h\"  // simple\n"

HazardyKnusperkeks wrote:
> yusuke-kadowaki wrote:
> > HazardyKnusperkeks wrote:
> > > Interesting would be a comment which is split, do we continue to align, 
> > > or not?
> > Could you give me a specific example?
> Something like
> ```
> int foo = 2323234; // Comment
> int bar = 52323;   // This is a very long comment, ...
>// which is wrapped around.
> 
> int x = 2; // Is this still aligned?
> ```
> You may need to make the comment longer or reduce the column limit, as often 
> used for testing the wrapping behavior.
Added that case with some column limits. I think it's working as expected. What 
do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

In D133082#3764256 , @aaron.ballman 
wrote:

> We use environment variables in several other places within the driver, but I 
> am a bit skittish about adding new ones to correspond to feature flags as 
> that seems to be somewhat novel. I think it'd be a good idea to get more 
> buy-in with an RFC on Discourse for more broad awareness.
>
> If we go this route, we definitely need user-facing documentation that 
> explains what's going on. I don't think we have anything corresponding to 
> https://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html, but having 
> such a page for Clang would be a good idea (even if the page is incomplete).

Okay, fair!

We will have an RFC about this.




Comment at: clang/lib/Driver/Driver.cpp:5427
+   ? A->getValue()
+   : 
std::getenv("CLANG_CRASH_DIAGNOSTICS_DIR");
+  if (CrashDirectory) {

aaron.ballman wrote:
> This should use `llvm::sys::Process::GetEnv()` so it properly handles text 
> encodings. CC @tahonermann @cor3ntin for text encoding awareness.
Okay, makes sense. But be aware that we have multiple places in the llvm 
project where we are using std::getenv to retrieve a filesystem path variable. 
There is another one within clang itself as well (`CLANG_MODULE_CACHE_PATH`, 
which is where I took inspiration from).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5427
+   ? A->getValue()
+   : 
std::getenv("CLANG_CRASH_DIAGNOSTICS_DIR");
+  if (CrashDirectory) {

mizvekov wrote:
> aaron.ballman wrote:
> > This should use `llvm::sys::Process::GetEnv()` so it properly handles text 
> > encodings. CC @tahonermann @cor3ntin for text encoding awareness.
> Okay, makes sense. But be aware that we have multiple places in the llvm 
> project where we are using std::getenv to retrieve a filesystem path 
> variable. There is another one within clang itself as well 
> (`CLANG_MODULE_CACHE_PATH`, which is where I took inspiration from).
Yeah, I noticed the same thing when I was looking around. Every one of those is 
a bug! It likely works fine for Linux where UTF-8 is common, but they will all 
fail on Windows where UTF-16 is used for the OS but `::getenv()` uses the C 
locale. Any sort of non-ASCII characters in the file path are likely not going 
to do the right thing in that case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:7671
+ AllArgs, CallType))
+return true;
+

yihanaa wrote:
> rjmccall wrote:
> > yihanaa wrote:
> > > rjmccall wrote:
> > > > You can just pull the argument expressions out of the `CallExpr`; you 
> > > > don't need to call `GatherArgumentsForCall`.
> > > > You can just pull the argument expressions out of the `CallExpr`; you 
> > > > don't need to call `GatherArgumentsForCall`.
> > > 
> > > This GatherArgumentsForCall  was used to do the common sema checking and 
> > > emit warning, like './main.cpp:5:40: warning: passing 'volatile char *' 
> > > to parameter of type 'const void *' discards qualifiers 
> > > [-Wincompatible-pointer-types-discards-qualifiers]' hahaha, for this is a 
> > > common case, I also think  GatherArgumentsForCall is not a good choice
> > > , so I try to find a replacement, e.g. ImpCastExprToType or other ways, 
> > > what do you think about?
> > `convertArgumentToType` should trigger any useful warnings in the second 
> > and third arguments.  For the first, I don't actually think there are any 
> > warnings we care about.
> I'm sorry John, I can't find `convertArgumentToType ` in clang, did you mean 
> `ConvertArgumentsForCall` or `ImpCastExprToType`. we can't use 
> `ConvertArgumentsForCall `, because `ConvertArgumentsForCall ` has checked if 
> current CallExpr calling a builtin function with custom sema checking, it 
> will do nothing and return.
Oh, sorry, it's a helper function in Apple's fork that we added for the ptrauth 
builtins but haven't upstreamed yet.  Feel free to add it yourself, at the top 
of the file right after `checkArgCount`:

```
static bool convertArgumentToType(Sema &S, Expr *&Value, QualType Ty) {
  if (Value->isTypeDependent())
return false;

  InitializedEntity Entity =
InitializedEntity::InitializeParameter(S.Context, Ty, false);
  ExprResult Result =
S.PerformCopyInitialization(Entity, SourceLocation(), Value);
  if (Result.isInvalid())
return true;
  Value = Result.get();
  return false;
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:7671
+ AllArgs, CallType))
+return true;
+

rjmccall wrote:
> yihanaa wrote:
> > rjmccall wrote:
> > > yihanaa wrote:
> > > > rjmccall wrote:
> > > > > You can just pull the argument expressions out of the `CallExpr`; you 
> > > > > don't need to call `GatherArgumentsForCall`.
> > > > > You can just pull the argument expressions out of the `CallExpr`; you 
> > > > > don't need to call `GatherArgumentsForCall`.
> > > > 
> > > > This GatherArgumentsForCall  was used to do the common sema checking 
> > > > and emit warning, like './main.cpp:5:40: warning: passing 'volatile 
> > > > char *' to parameter of type 'const void *' discards qualifiers 
> > > > [-Wincompatible-pointer-types-discards-qualifiers]' hahaha, for this is 
> > > > a common case, I also think  GatherArgumentsForCall is not a good choice
> > > > , so I try to find a replacement, e.g. ImpCastExprToType or other ways, 
> > > > what do you think about?
> > > `convertArgumentToType` should trigger any useful warnings in the second 
> > > and third arguments.  For the first, I don't actually think there are any 
> > > warnings we care about.
> > I'm sorry John, I can't find `convertArgumentToType ` in clang, did you 
> > mean `ConvertArgumentsForCall` or `ImpCastExprToType`. we can't use 
> > `ConvertArgumentsForCall `, because `ConvertArgumentsForCall ` has checked 
> > if current CallExpr calling a builtin function with custom sema checking, 
> > it will do nothing and return.
> Oh, sorry, it's a helper function in Apple's fork that we added for the 
> ptrauth builtins but haven't upstreamed yet.  Feel free to add it yourself, 
> at the top of the file right after `checkArgCount`:
> 
> ```
> static bool convertArgumentToType(Sema &S, Expr *&Value, QualType Ty) {
>   if (Value->isTypeDependent())
> return false;
> 
>   InitializedEntity Entity =
> InitializedEntity::InitializeParameter(S.Context, Ty, false);
>   ExprResult Result =
> S.PerformCopyInitialization(Entity, SourceLocation(), Value);
>   if (Result.isInvalid())
> return true;
>   Value = Result.get();
>   return false;
> }
> ```
wow,cool! Thanks John


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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

[PATCH] D133066: fix a typo in comment of AddConversionCandidate

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The existing comment is correct according to my copy of the C++11 standard, but 
the standard has changed since C++11 and those words no longer appear in 
http://eel.is/c++draft/dcl.init.ref#5. Some more investigation is needed, but I 
suspect the correct action to take here is to update the comment based on the 
current standards wording or to see if there's a bug because the code does not 
match the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133066

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for working on this! It's really great to get the crash report as an 
artifact.




Comment at: libcxx/test/libcxx/crash.sh.cpp:15
+
+#pragma clang __debug parser_crash

The libc++ build seems to be green. I assume it was intended to be red so we 
can validate the artifact is available in the CI.



Comment at: libcxx/utils/ci/buildkite-pipeline.yml:377
 LLVM_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer-${LLVM_HEAD_VERSION}"
+CLANG_CRASH_DIAGNOSTICS_DIR: "crash_diagnostics"
 agents:

Are relative directories allowed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added a comment.

For the Linux kernel, we're only building v5+ continuously.

It looks like the Linux kernel supports v4+ and v3m (for Acorn Risc-PC (Intel 
StrongARM(R) SA-110)). We've never been able to build that target, and it's not 
high ROI to do so.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/Makefile#n74

> GCC no longer supports Arm architecture prior to v4 so it is likely the 
> alternative of adding support for v3 is not worth it.

Oh, looks like the kernel already has a check for (6 <= GCC < 9.1), so there's 
no risk of breakage on our side.  Thanks for the heads up though!
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2abd6e34fcf3bd9f9ffafcaa47cdc3ed443f9add

Consider amending the commit message to mention the specific version of GCC 
that support for v3- was removed from.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133109

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: libcxx/test/libcxx/crash.sh.cpp:15
+
+#pragma clang __debug parser_crash

Mordante wrote:
> The libc++ build seems to be green. I assume it was intended to be red so we 
> can validate the artifact is available in the CI.
If I made it red, this test would fail in one the jobs from the early group, 
and we would never run the bootstraping job :(

But making it green does not stop it from exporting the artifact, you can go in 
there in the build results and have a look!



Comment at: libcxx/utils/ci/buildkite-pipeline.yml:377
 LLVM_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer-${LLVM_HEAD_VERSION}"
+CLANG_CRASH_DIAGNOSTICS_DIR: "crash_diagnostics"
 agents:

Mordante wrote:
> Are relative directories allowed?
Yes, it already worked with the flag.

You can check that it's working, even though the bootstrapping pipeline is 
green, it still exports the artifacts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133043: [clangd] Fix tests for implicit C function declaration

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

LGTM, but someone with more clangd experience should do the final sign off 
(seeing as how I caused the issue in the first place, sorry about that!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133043

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

[PATCH] D125655: [HLSL] add -P option for dxc mode.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 457313.
python3kgae added a comment.

Not need to send -P and -Fi to cc1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125655

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/test/Driver/dxc_P.hlsl

Index: clang/test/Driver/dxc_P.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_P.hlsl
@@ -0,0 +1,16 @@
+// RUN: %clang_dxc -Tlib_6_7 -P -### %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7  -P -Fi"a.txt" -### %s 2>&1 | FileCheck %s --check-prefix=FI
+// RUN: %clang_dxc -Tlib_6_7  -Fo b.txt -P -Fi"a.txt" -### %s 2>&1 | FileCheck %s --check-prefix=WARNING
+
+// Make sure -P option flag which translated into "-E" + "-o" "dxc_P.i".
+// CHECK:"-E"
+// CHECK-SAME: "-o" "dxc_P.i"
+
+// Make sure -Fi updated -o.
+// FI:"-E"
+// FI-SAME:"-o" "a.txt"
+
+// Make sure got warning when -Fo and -P at same time.
+// WARNING: warning: output compiler options like -Fo ignored with Preprocess [-Woption-ignored]
+// WARNING:"-E"
+// WARNING-SAME:"-o" "a.txt"
Index: clang/lib/Driver/ToolChains/HLSL.cpp
===
--- clang/lib/Driver/ToolChains/HLSL.cpp
+++ clang/lib/Driver/ToolChains/HLSL.cpp
@@ -177,6 +177,9 @@
   }
 
   if (DAL->hasArg(options::OPT_o)) {
+if (DAL->hasArg(options::OPT__SLASH_P))
+  getDriver().Diag(diag::warn_drv_dxc_ignore_output_for_preprocess);
+
 // When run the whole pipeline.
 if (!DAL->hasArg(options::OPT_emit_llvm))
   // Emit obj if write to file.
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -5455,7 +5455,8 @@
 
   llvm::PrettyStackTraceString CrashInfo("Computing output path");
   // Output to a user requested destination?
-  if (AtTopLevel && !isa(JA) && !isa(JA)) {
+  if (AtTopLevel && !isa(JA) && !isa(JA) &&
+  !(IsDXCMode() && C.getArgs().hasArg(options::OPT__SLASH_P))) {
 if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))
   return C.addResultFile(FinalOutput->getValue(), &JA);
   }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6381,6 +6381,9 @@
 class CLFlag : Option<["/", "-"], name, KIND_FLAG>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
+class CLDXCFlag : Option<["/", "-"], name, KIND_FLAG>,
+  Group, Flags<[CLDXCOption, NoXarchOption]>;
+
 class CLCompileFlag : Option<["/", "-"], name, KIND_FLAG>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
@@ -6393,6 +6396,9 @@
 class CLCompileJoined : Option<["/", "-"], name, KIND_JOINED>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
+class CLDXCCompileJoined : Option<["/", "-"], name, KIND_JOINED>,
+  Group, Flags<[CLDXCOption, NoXarchOption]>;
+
 class CLIgnoredJoined : Option<["/", "-"], name, KIND_JOINED>,
   Group, Flags<[CLOption, NoXarchOption, HelpHidden]>;
 
@@ -6662,7 +6668,7 @@
   HelpText<"Set output executable file name">,
   MetaVarName<"">;
 def _SLASH_Fe_COLON : CLJoined<"Fe:">, Alias<_SLASH_Fe>;
-def _SLASH_Fi : CLCompileJoined<"Fi">,
+def _SLASH_Fi : CLDXCCompileJoined<"Fi">,
   HelpText<"Set preprocess output file name (with /P)">,
   MetaVarName<"">;
 def _SLASH_Fo : CLCompileJoined<"Fo">,
@@ -6697,7 +6703,7 @@
 def _SLASH_o : CLJoinedOrSeparate<"o">,
   HelpText<"Deprecated (set output file name); use /Fe or /Fe">,
   MetaVarName<"">;
-def _SLASH_P : CLFlag<"P">, HelpText<"Preprocess to file">;
+def _SLASH_P : CLDXCFlag<"P">, HelpText<"Preprocess to file">;
 def _SLASH_permissive : CLFlag<"permissive">,
   HelpText<"Enable some non conforming code to compile">;
 def _SLASH_permissive_ : CLFlag<"permissive-">,
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -666,6 +666,9 @@
   "invalid profile : %0">;
 def err_drv_dxc_missing_target_profile : Error<
   "target profile option (-T) is missing">;
+def warn_drv_dxc_ignore_output_for_preprocess : Warning<
+  "output compiler options like -Fo ignored with Preprocess">,
+  InGroup;
 
 def err_drv_invalid_range_dxil_validator_version : Error<
   "invalid validator version : %0\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits