Merged master:65c489852c4e into amd-gfx:01c4f02f038f

Local branch amd-gfx 01c4f02 Merge remote-tracking branch 'llvm.org/master' into amd-gfx
Remote branch master 65c4898 [gn build] Port 9ca6fc4

clang-tools-extra/clang-tidy/altera/AlteraTidyModule.cpp

@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "KernelNameRestrictionCheck.h"
 #include "StructPackAlignCheck.h"
 
 using namespace clang::ast_matchers;
@@ -20,6 +21,8 @@ namespace altera {
 class AlteraModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.registerCheck<KernelNameRestrictionCheck>(
+        "altera-kernel-name-restriction");
     CheckFactories.registerCheck<StructPackAlignCheck>(
         "altera-struct-pack-align");
   }

clang-tools-extra/clang-tidy/altera/CMakeLists.txt

@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
 
 add_clang_library(clangTidyAlteraModule
   AlteraTidyModule.cpp
+  KernelNameRestrictionCheck.cpp
   StructPackAlignCheck.cpp
 
   LINK_LIBS

clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.cpp

@@ -0,0 +1,107 @@
+//===--- KernelNameRestrictionCheck.cpp - clang-tidy ----------------------===//
+//
+// 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 "KernelNameRestrictionCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include <string>
+#include <vector>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace altera {
+
+namespace {
+
+class KernelNameRestrictionPPCallbacks : public PPCallbacks {
+public:
+  explicit KernelNameRestrictionPPCallbacks(ClangTidyCheck &Check,
+                                            const SourceManager &SM)
+      : Check(Check), SM(SM) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+                          StringRef FileName, bool IsAngled,
+                          CharSourceRange FileNameRange, const FileEntry *File,
+                          StringRef SearchPath, StringRef RelativePath,
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override;
+
+  void EndOfMainFile() override;
+
+private:
+  /// Returns true if the name of the file with path FilePath is 'kernel.cl',
+  /// 'verilog.cl', or 'vhdl.cl'. The file name check is case insensitive.
+  bool FileNameIsRestricted(StringRef FilePath);
+
+  struct IncludeDirective {
+    SourceLocation Loc; // Location in the include directive.
+    StringRef FileName; // Filename as a string.
+  };
+
+  std::vector<IncludeDirective> IncludeDirectives;
+  ClangTidyCheck &Check;
+  const SourceManager &SM;
+};
+
+} // namespace
+
+void KernelNameRestrictionCheck::registerPPCallbacks(const SourceManager &SM,
+                                                     Preprocessor *PP,
+                                                     Preprocessor *) {
+  PP->addPPCallbacks(
+      std::make_unique<KernelNameRestrictionPPCallbacks>(*this, SM));
+}
+
+void KernelNameRestrictionPPCallbacks::InclusionDirective(
+    SourceLocation HashLoc, const Token &, StringRef FileName, bool,
+    CharSourceRange, const FileEntry *, StringRef, StringRef, const Module *,
+    SrcMgr::CharacteristicKind) {
+  IncludeDirective ID = {HashLoc, FileName};
+  IncludeDirectives.push_back(std::move(ID));
+}
+
+bool KernelNameRestrictionPPCallbacks::FileNameIsRestricted(
+    StringRef FileName) {
+  return FileName.equals_lower("kernel.cl") ||
+         FileName.equals_lower("verilog.cl") ||
+         FileName.equals_lower("vhdl.cl");
+}
+
+void KernelNameRestrictionPPCallbacks::EndOfMainFile() {
+
+  // Check main file for restricted names.
+  const FileEntry *Entry = SM.getFileEntryForID(SM.getMainFileID());
+  StringRef FileName = llvm::sys::path::filename(Entry->getName());
+  if (FileNameIsRestricted(FileName))
+    Check.diag(SM.getLocForStartOfFile(SM.getMainFileID()),
+               "compiling '%0' may cause additional compilation errors due "
+               "to the name of the kernel source file; consider renaming the "
+               "included kernel source file")
+        << FileName;
+
+  if (IncludeDirectives.empty())
+    return;
+
+  // Check included files for restricted names.
+  for (const IncludeDirective &ID : IncludeDirectives) {
+    StringRef FileName = llvm::sys::path::filename(ID.FileName);
+    if (FileNameIsRestricted(FileName))
+      Check.diag(ID.Loc,
+                 "including '%0' may cause additional compilation errors due "
+                 "to the name of the kernel source file; consider renaming the "
+                 "included kernel source file")
+          << FileName;
+  }
+}
+
+} // namespace altera
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/altera/KernelNameRestrictionCheck.h

@@ -0,0 +1,35 @@
+//===--- KernelNameRestrictionCheck.h - clang-tidy --------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_KERNEL_NAME_RESTRICTION_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_KERNEL_NAME_RESTRICTION_CHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace altera {
+
+/// Finds kernel files and include directives whose filename is `kernel.cl`,
+/// `Verilog.cl`, or `VHDL.cl`.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/altera-kernel-name-restriction.html
+class KernelNameRestrictionCheck : public ClangTidyCheck {
+public:
+  KernelNameRestrictionCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *) override;
+};
+
+} // namespace altera
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ALTERA_KERNEL_NAME_RESTRICTION_CHECK_H

clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

@@ -754,7 +754,6 @@ IdentifierNamingCheck::getStyleForFile(StringRef FileName) const {
     assert(It.second);
     return It.first->getValue();
   }
-  assert(false);
   // Default construction gives an empty style.
   auto It = NamingStylesCache.try_emplace(Parent);
   assert(It.second);

clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp

@@ -40,7 +40,8 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Core/Replacement.h"
-#include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/MapVector.h"
+#include "llvm/ADT/STLExtras.h"
 #include <cassert>
 #include <string>
 
@@ -57,11 +58,27 @@ class PopulateSwitch : public Tweak {
   }
 
 private:
+  class ExpectedCase {
+  public:
+    ExpectedCase(const EnumConstantDecl *Decl) : Data(Decl, false) {}
+    bool isCovered() const { return Data.getInt(); }
+    void setCovered(bool Val = true) { Data.setInt(Val); }
+    const EnumConstantDecl *getEnumConstant() const {
+      return Data.getPointer();
+    }
+
+  private:
+    llvm::PointerIntPair<const EnumConstantDecl *, 1, bool> Data;
+  };
+
   const DeclContext *DeclCtx = nullptr;
   const SwitchStmt *Switch = nullptr;
   const CompoundStmt *Body = nullptr;
   const EnumType *EnumT = nullptr;
   const EnumDecl *EnumD = nullptr;
+  // Maps the Enum values to the EnumConstantDecl and a bool signifying if its
+  // covered in the switch.
+  llvm::MapVector<llvm::APSInt, ExpectedCase> ExpectedCases;
 };
 
 REGISTER_TWEAK(PopulateSwitch)
@@ -112,21 +129,34 @@ bool PopulateSwitch::prepare(const Selection &Sel) {
   if (!EnumD)
     return false;
 
-  // We trigger if there are fewer cases than enum values (and no case covers
-  // multiple values). This guarantees we'll have at least one case to insert.
-  // We don't yet determine what the cases are, as that means evaluating
-  // expressions.
-  auto I = EnumD->enumerator_begin();
-  auto E = EnumD->enumerator_end();
+  // We trigger if there are any values in the enum that aren't covered by the
+  // switch.
+
+  ASTContext &Ctx = Sel.AST->getASTContext();
+
+  unsigned EnumIntWidth = Ctx.getIntWidth(QualType(EnumT, 0));
+  bool EnumIsSigned = EnumT->isSignedIntegerOrEnumerationType();
 
-  for (const SwitchCase *CaseList = Switch->getSwitchCaseList();
-       CaseList && I != E; CaseList = CaseList->getNextSwitchCase(), I++) {
+  auto Normalize = [&](llvm::APSInt Val) {
+    Val = Val.extOrTrunc(EnumIntWidth);
+    Val.setIsSigned(EnumIsSigned);
+    return Val;
+  };
+
+  for (auto *EnumConstant : EnumD->enumerators()) {
+    ExpectedCases.insert(
+        std::make_pair(Normalize(EnumConstant->getInitVal()), EnumConstant));
+  }
+
+  for (const SwitchCase *CaseList = Switch->getSwitchCaseList(); CaseList;
+       CaseList = CaseList->getNextSwitchCase()) {
     // Default likely intends to cover cases we'd insert.
     if (isa<DefaultStmt>(CaseList))
       return false;
 
     const CaseStmt *CS = cast<CaseStmt>(CaseList);
-    // Case statement covers multiple values, so just counting doesn't work.
+
+    // GNU range cases are rare, we don't support them.
     if (CS->caseStmtIsGNURange())
       return false;
 
@@ -135,48 +165,36 @@ bool PopulateSwitch::prepare(const Selection &Sel) {
     const ConstantExpr *CE = dyn_cast<ConstantExpr>(CS->getLHS());
     if (!CE || CE->isValueDependent())
       return false;
+
+    // Unsure if this case could ever come up, but prevents an unreachable
+    // executing in getResultAsAPSInt.
+    if (CE->getResultStorageKind() == ConstantExpr::RSK_None)
+      return false;
+    auto Iter = ExpectedCases.find(Normalize(CE->getResultAsAPSInt()));
+    if (Iter != ExpectedCases.end())
+      Iter->second.setCovered();
   }
 
-  // Only suggest tweak if we have more enumerators than cases.
-  return I != E;
+  return !llvm::all_of(ExpectedCases,
+                       [](auto &Pair) { return Pair.second.isCovered(); });
 }
 
 Expected<Tweak::Effect> PopulateSwitch::apply(const Selection &Sel) {
   ASTContext &Ctx = Sel.AST->getASTContext();
 
-  // Get the enum's integer width and signedness, for adjusting case literals.
-  unsigned EnumIntWidth = Ctx.getIntWidth(QualType(EnumT, 0));
-  bool EnumIsSigned = EnumT->isSignedIntegerOrEnumerationType();
-
-  llvm::SmallSet<llvm::APSInt, 32> ExistingEnumerators;
-  for (const SwitchCase *CaseList = Switch->getSwitchCaseList(); CaseList;
-       CaseList = CaseList->getNextSwitchCase()) {
-    const CaseStmt *CS = cast<CaseStmt>(CaseList);
-    assert(!CS->caseStmtIsGNURange());
-    const ConstantExpr *CE = cast<ConstantExpr>(CS->getLHS());
-    assert(!CE->isValueDependent());
-    llvm::APSInt Val = CE->getResultAsAPSInt();
-    Val = Val.extOrTrunc(EnumIntWidth);
-    Val.setIsSigned(EnumIsSigned);
-    ExistingEnumerators.insert(Val);
-  }
-
   SourceLocation Loc = Body->getRBracLoc();
   ASTContext &DeclASTCtx = DeclCtx->getParentASTContext();
 
-  std::string Text;
-  for (EnumConstantDecl *Enumerator : EnumD->enumerators()) {
-    if (ExistingEnumerators.contains(Enumerator->getInitVal()))
+  llvm::SmallString<256> Text;
+  for (auto &EnumConstant : ExpectedCases) {
+    // Skip any enum constants already covered
+    if (EnumConstant.second.isCovered())
       continue;
 
-    Text += "case ";
-    Text += getQualification(DeclASTCtx, DeclCtx, Loc, EnumD);
-    if (EnumD->isScoped()) {
-      Text += EnumD->getName();
-      Text += "::";
-    }
-    Text += Enumerator->getName();
-    Text += ":";
+    Text.append({"case ", getQualification(DeclASTCtx, DeclCtx, Loc, EnumD)});
+    if (EnumD->isScoped())
+      Text.append({EnumD->getName(), "::"});
+    Text.append({EnumConstant.second.getEnumConstant()->getName(), ":"});
   }
 
   assert(!Text.empty() && "No enumerators to insert!");

clang-tools-extra/clangd/unittests/TweakTests.cpp

@@ -2980,6 +2980,18 @@ TEST_F(PopulateSwitchTest, Test) {
             void function() { switch (ns::A) {case ns::A:break;} }
           )"",
       },
+      {
+          // Duplicated constant names
+          Function,
+          R""(enum Enum {A,B,b=B}; ^switch (A) {})"",
+          R""(enum Enum {A,B,b=B}; switch (A) {case A:case B:break;})"",
+      },
+      {
+          // Duplicated constant names all in switch
+          Function,
+          R""(enum Enum {A,B,b=B}; ^switch (A) {case A:case B:break;})"",
+          "unavailable",
+      },
   };
 
   for (const auto &Case : Cases) {

clang-tools-extra/docs/ReleaseNotes.rst

@@ -85,6 +85,12 @@ New modules
 New checks
 ^^^^^^^^^^
 
+- New :doc:`altera-kernel-name-restriction
+  <clang-tidy/checks/altera-kernel-name-restriction>` check.
+
+  Finds kernel files and include directives whose filename is `kernel.cl`,
+  `Verilog.cl`, or `VHDL.cl`.
+
 - New :doc:`altera-struct-pack-align
   <clang-tidy/checks/altera-struct-pack-align>` check.
 

clang-tools-extra/docs/clang-tidy/checks/altera-kernel-name-restriction.rst

@@ -0,0 +1,15 @@
+.. title:: clang-tidy - altera-kernel-name-restriction
+
+altera-kernel-name-restriction
+==============================
+
+Finds kernel files and include directives whose filename is `kernel.cl`,
+`Verilog.cl`, or `VHDL.cl`. The check is case insensitive.
+
+Such kernel file names cause the offline compiler to generate intermediate
+design files that have the same names as certain internal files, which
+leads to a compilation error.
+
+Based on the `Guidelines for Naming the Kernel` section in the
+`Intel FPGA SDK for OpenCL Pro Edition: Programming Guide
+<https://www.intel.com/content/www/us/en/programmable/documentation/mwh1391807965224.html#ewa1412973930963>`_.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -30,6 +30,7 @@ Clang-Tidy Checks
    `abseil-time-comparison <abseil-time-comparison.html>`_, "Yes"
    `abseil-time-subtraction <abseil-time-subtraction.html>`_, "Yes"
    `abseil-upgrade-duration-conversions <abseil-upgrade-duration-conversions.html>`_, "Yes"
+   `altera-kernel-name-restriction <altera-kernel-name-restriction.html>`_,
    `altera-struct-pack-align <altera-struct-pack-align.html>`_,
    `android-cloexec-accept <android-cloexec-accept.html>`_, "Yes"
    `android-cloexec-accept4 <android-cloexec-accept4.html>`_,

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/Verilog.cl

@@ -0,0 +1 @@
+const int VERILOGINT = 2;

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.cl

@@ -0,0 +1 @@
+const int KERNELINT = 1;

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/kernel.h

@@ -0,0 +1 @@
+const int KERNELINT3 = 1;

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/other_Verilog.cl

@@ -0,0 +1 @@
+const int OTHERVERILOGINT = 2;

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherdir/vhdl.cl

@@ -0,0 +1 @@
+const int OTHERDIRVHDLINT = 3;

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/otherthing.cl

@@ -0,0 +1 @@
+const int OTHERTHINGINT = 1;

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/dir/kernel.cl

@@ -0,0 +1 @@
+const int SOMEDIRKERNELINT = 1;

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/kernel.cl/foo.h

@@ -0,0 +1 @@
+int SOME_KERNEL_FOO_INT = 0;

clang-tools-extra/test/clang-tidy/checkers/Inputs/altera-kernel-name-restriction/some/verilog.cl/foo.h

@@ -0,0 +1 @@
+int SOME_VERILOG_FOO_INT = 0;