merge main into amd-staging (llvm#1936)

Author: ronlieb

clang-tools-extra/clang-include-fixer/IncludeFixer.cpp

@@ -89,8 +89,7 @@ bool IncludeFixerActionFactory::runInvocation(
   assert(Invocation->getFrontendOpts().Inputs.size() == 1);
 
   // Set up Clang.
-  clang::CompilerInstance Compiler(PCHContainerOps);
-  Compiler.setInvocation(std::move(Invocation));
+  CompilerInstance Compiler(std::move(Invocation), std::move(PCHContainerOps));
   Compiler.setFileManager(Files);
 
   // Create the compiler's actual diagnostics engine. We want to drop all

clang-tools-extra/clangd/Compiler.cpp

@@ -145,9 +145,7 @@ prepareCompilerInstance(std::unique_ptr<clang::CompilerInvocation> CI,
         CI->getFrontendOpts().Inputs[0].getFile(), Buffer.get());
   }
 
-  auto Clang = std::make_unique<CompilerInstance>(
-      std::make_shared<PCHContainerOperations>());
-  Clang->setInvocation(std::move(CI));
+  auto Clang = std::make_unique<CompilerInstance>(std::move(CI));
   Clang->createDiagnostics(*VFS, &DiagsClient, false);
 
   if (auto VFSWithRemapping = createVFSFromCompilerInvocation(

clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

@@ -618,14 +618,14 @@ TEST_F(PragmaIncludeTest, ExportInUnnamedBuffer) {
                  llvm::MemoryBuffer::getMemBufferCopy(Extra.getValue(),
                                                       /*BufferName=*/""));
 
-  auto Clang = std::make_unique<CompilerInstance>(
-      std::make_shared<PCHContainerOperations>());
-  Clang->createDiagnostics(*VFS);
+  auto DiagOpts = llvm::makeIntrusiveRefCnt<DiagnosticOptions>();
+  auto Diags = CompilerInstance::createDiagnostics(*VFS, DiagOpts.get());
+  auto Invocation = std::make_unique<CompilerInvocation>();
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(*Invocation, {Filename.data()},
+                                                 *Diags, "clang"));
 
-  Clang->setInvocation(std::make_unique<CompilerInvocation>());
-  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(
-      Clang->getInvocation(), {Filename.data()}, Clang->getDiagnostics(),
-      "clang"));
+  auto Clang = std::make_unique<CompilerInstance>(std::move(Invocation));
+  Clang->createDiagnostics(*VFS);
 
   auto *FM = Clang->createFileManager(VFS);
   ASSERT_TRUE(Clang->ExecuteAction(*Inputs.MakeAction()));

clang/docs/ReleaseNotes.rst

@@ -389,6 +389,7 @@ Improvements to Clang's diagnostics
   under the subgroup ``-Wunsafe-buffer-usage-in-libc-call``.
 - Diagnostics on chained comparisons (``a < b < c``) are now an error by default. This can be disabled with
   ``-Wno-error=parentheses``.
+- Similarly, fold expressions over a comparison operator are now an error by default.
 - Clang now better preserves the sugared types of pointers to member.
 - Clang now better preserves the presence of the template keyword with dependent
   prefixes.

clang/include/clang/Basic/Diagnostic.h

@@ -1429,6 +1429,25 @@ operator<<(const StreamingDiagnostic &DB, T *DC) {
   return DB;
 }
 
+// Convert scoped enums to their underlying type, so that we don't have
+// clutter the emitting code with `llvm::to_underlying()`.
+// We also need to disable implicit conversion for the first argument,
+// because classes that derive from StreamingDiagnostic define their own
+// templated operator<< that accept a wide variety of types, leading
+// to ambiguity.
+template <typename T, typename U,
+          typename UnderlyingU = typename std::enable_if_t<
+              std::is_enum_v<std::remove_reference_t<U>>,
+              std::underlying_type<std::remove_reference_t<U>>>::type>
+inline std::enable_if_t<
+    std::is_same_v<std::remove_const_t<T>, StreamingDiagnostic> &&
+        !std::is_convertible_v<U, UnderlyingU>,
+    const StreamingDiagnostic &>
+operator<<(const T &DB, U &&SE) {
+  DB << llvm::to_underlying(SE);
+  return DB;
+}
+
 inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
                                              SourceLocation L) {
   DB.AddSourceRange(CharSourceRange::getTokenRange(L));

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7204,6 +7204,11 @@ def warn_consecutive_comparison : Warning<
   "chained comparison 'X %0 Y %1 Z' does not behave the same as a mathematical expression">,
   InGroup<Parentheses>, DefaultError;
 
+def warn_comparison_in_fold_expression : Warning<
+  "comparison in fold expression would evaluate to '(X %0 Y) %0 Z' "
+  "which does not behave the same as a mathematical expression">,
+  InGroup<Parentheses>, DefaultError;
+
 def warn_enum_constant_in_bool_context : Warning<
   "converting the enum constant to a boolean">,
   InGroup<IntInBoolContext>, DefaultIgnore;

clang/include/clang/Frontend/CompilerInstance.h

@@ -204,6 +204,8 @@ class CompilerInstance : public ModuleLoader {
   void operator=(const CompilerInstance &) = delete;
 public:
   explicit CompilerInstance(
+      std::shared_ptr<CompilerInvocation> Invocation =
+          std::make_shared<CompilerInvocation>(),
       std::shared_ptr<PCHContainerOperations> PCHContainerOps =
           std::make_shared<PCHContainerOperations>(),
       ModuleCache *ModCache = nullptr);
@@ -251,18 +253,10 @@ class CompilerInstance : public ModuleLoader {
   /// @name Compiler Invocation and Options
   /// @{
 
-  bool hasInvocation() const { return Invocation != nullptr; }
-
-  CompilerInvocation &getInvocation() {
-    assert(Invocation && "Compiler instance has no invocation!");
-    return *Invocation;
-  }
+  CompilerInvocation &getInvocation() { return *Invocation; }
 
   std::shared_ptr<CompilerInvocation> getInvocationPtr() { return Invocation; }
 
-  /// setInvocation - Replace the current invocation.
-  void setInvocation(std::shared_ptr<CompilerInvocation> Value);
-
   /// Indicates whether we should (re)build the global module index.
   bool shouldBuildGlobalModuleIndex() const;
 

clang/include/clang/Sema/Sema.h

@@ -220,11 +220,6 @@ enum class AssignmentAction {
   Casting,
   Passing_CFAudited
 };
-inline const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
-                                             const AssignmentAction &AA) {
-  DB << llvm::to_underlying(AA);
-  return DB;
-}
 
 namespace threadSafety {
 class BeforeSet;
@@ -7220,13 +7215,15 @@ class Sema final : public SemaBase {
   ExprResult ActOnBinOp(Scope *S, SourceLocation TokLoc, tok::TokenKind Kind,
                         Expr *LHSExpr, Expr *RHSExpr);
   ExprResult BuildBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc,
-                        Expr *LHSExpr, Expr *RHSExpr);
+                        Expr *LHSExpr, Expr *RHSExpr,
+                        bool ForFoldExpression = false);
 
   /// CreateBuiltinBinOp - Creates a new built-in binary operation with
   /// operator @p Opc at location @c TokLoc. This routine only supports
   /// built-in operations; ActOnBinOp handles overloaded operators.
   ExprResult CreateBuiltinBinOp(SourceLocation OpLoc, BinaryOperatorKind Opc,
-                                Expr *LHSExpr, Expr *RHSExpr);
+                                Expr *LHSExpr, Expr *RHSExpr,
+                                bool ForFoldExpression = false);
   void LookupBinOp(Scope *S, SourceLocation OpLoc, BinaryOperatorKind Opc,
                    UnresolvedSetImpl &Functions);
 
@@ -15471,12 +15468,6 @@ void Sema::PragmaStack<Sema::AlignPackInfo>::Act(SourceLocation PragmaLocation,
                                                  llvm::StringRef StackSlotLabel,
                                                  AlignPackInfo Value);
 
-inline const StreamingDiagnostic &
-operator<<(const StreamingDiagnostic &DB, Sema::StringEvaluationContext Ctx) {
-  DB << llvm::to_underlying(Ctx);
-  return DB;
-}
-
 } // end namespace clang
 
 #endif

clang/lib/AST/ODRDiagsEmitter.cpp

@@ -461,10 +461,8 @@ bool ODRDiagsEmitter::diagnoseSubMismatchObjCMethod(
   }
   if (FirstMethod->getImplementationControl() !=
       SecondMethod->getImplementationControl()) {
-    DiagError(ControlLevel)
-        << llvm::to_underlying(FirstMethod->getImplementationControl());
-    DiagNote(ControlLevel) << llvm::to_underlying(
-        SecondMethod->getImplementationControl());
+    DiagError(ControlLevel) << FirstMethod->getImplementationControl();
+    DiagNote(ControlLevel) << SecondMethod->getImplementationControl();
     return true;
   }
   if (FirstMethod->isThisDeclarationADesignatedInitializer() !=

clang/lib/CIR/CodeGen/CIRGenStmtOpenACCLoop.cpp

@@ -13,17 +13,103 @@
 #include "CIRGenBuilder.h"
 #include "CIRGenFunction.h"
 #include "CIRGenOpenACCClause.h"
-#include "mlir/Dialect/OpenACC/OpenACC.h"
+
 #include "clang/AST/OpenACCClause.h"
 #include "clang/AST/StmtOpenACC.h"
 
+#include "mlir/Dialect/OpenACC/OpenACC.h"
+
 using namespace clang;
 using namespace clang::CIRGen;
 using namespace cir;
 using namespace mlir::acc;
 
 mlir::LogicalResult
 CIRGenFunction::emitOpenACCLoopConstruct(const OpenACCLoopConstruct &s) {
-  cgm.errorNYI(s.getSourceRange(), "OpenACC Loop Construct");
-  return mlir::failure();
+  mlir::Location start = getLoc(s.getSourceRange().getBegin());
+  mlir::Location end = getLoc(s.getSourceRange().getEnd());
+  llvm::SmallVector<mlir::Type> retTy;
+  llvm::SmallVector<mlir::Value> operands;
+  auto op = builder.create<LoopOp>(start, retTy, operands);
+
+  // TODO(OpenACC): In the future we are going to need to come up with a
+  // transformation here that can teach the acc.loop how to figure out the
+  // 'lowerbound', 'upperbound', and 'step'.
+  //
+  // -'upperbound' should fortunately be pretty easy as it should be
+  // in the initialization section of the cir.for loop. In Sema, we limit to
+  // just the forms 'Var = init', `Type Var = init`, or `Var = init` (where it
+  // is an operator= call)`.  However, as those are all necessary to emit for
+  // the init section of the for loop, they should be inside the initial
+  // cir.scope.
+  //
+  // -'upperbound' should be somewhat easy to determine. Sema is limiting this
+  // to: ==, <, >, !=,  <=, >= builtin operators, the overloaded 'comparison'
+  // operations, and member-call expressions.
+  //
+  // For the builtin comparison operators, we can pretty well deduce based on
+  // the comparison what the 'end' object is going to be, and the inclusive
+  // nature of it.
+  //
+  // For the overloaded operators, Sema will ensure that at least one side of
+  // the operator is the init variable, so we can deduce the comparison there
+  // too. The standard places no real bounds on WHAT the comparison operators do
+  // for a `RandomAccessIterator` however, so we'll have to just 'assume' they
+  // do the right thing? Note that this might be incrementing by a different
+  // 'object', not an integral, so it isn't really clear to me what we can do to
+  // determine the other side.
+  //
+  // Member-call expressions are the difficult ones. I don't think there is
+  // anything we can deduce from this to determine the 'end', so we might end up
+  // having to go back to Sema and make this ill-formed.
+  //
+  // HOWEVER: What ACC dialect REALLY cares about is the tripcount, which you
+  // cannot get (in the case of `RandomAccessIterator`) from JUST 'upperbound'
+  // and 'lowerbound'. We will likely have to provide a 'recipe' equivalent to
+  // `std::distance` instead.  In the case of integer/pointers, it is fairly
+  // simple to find: it is just the mathematical subtraction. Howver, in the
+  // case of `RandomAccessIterator`, we have to enable the use of `operator-`.
+  // FORTUNATELY the standard requires this to work correctly for
+  // `RandomAccessIterator`, so we don't have to implement a `std::distance`
+  // that loops through, like we would for a forward/etc iterator.
+  //
+  // 'step': Sema is currently allowing builtin ++,--, +=, -=, *=, /=, and =
+  // operators. Additionally, it allows the equivalent for the operator-call, as
+  // well as member-call.
+  //
+  // For builtin operators, we perhaps should refine the assignment here. It
+  // doesn't really help us know the 'step' count at all, but we could perhaps
+  // do one more step of analysis in Sema to allow something like Var = Var + 1.
+  // For the others, this should get us the step reasonably well.
+  //
+  // For the overloaded operators, we have the same problems as for
+  // 'upperbound', plus not really knowing what they do. Member-call expressions
+  // are again difficult, and we might want to reconsider allowing these in
+  // Sema.
+  //
+
+  // Emit all clauses.
+  {
+    mlir::OpBuilder::InsertionGuard guardCase(builder);
+    // Sets insertion point before the 'op', since every new expression needs to
+    // be before the operation.
+    builder.setInsertionPoint(op);
+    makeClauseEmitter(op, *this, builder, s.getDirectiveKind(),
+                      s.getDirectiveLoc())
+        .VisitClauseList(s.clauses());
+  }
+
+  mlir::LogicalResult stmtRes = mlir::success();
+  // Emit body.
+  {
+    mlir::Block &block = op.getRegion().emplaceBlock();
+    mlir::OpBuilder::InsertionGuard guardCase(builder);
+    builder.setInsertionPointToEnd(&block);
+    LexicalScope ls{*this, start, builder.getInsertionBlock()};
+
+    stmtRes = emitStmt(s.getLoop(), /*useCurrentScope=*/true);
+    builder.create<mlir::acc::YieldOp>(end);
+  }
+
+  return stmtRes;
 }