[PATCH] D112175: [NFC] Add llvm::StaticVector ADT

[James Player via Phabricator via llvm-commits]

jplayer-nv added a comment.

I don't see too many simple substitutions in target independent code, although this class has a SmallVector which never exceeds the inline storage (drop-in type substitution + with YAML binding works):

include\llvm\DebugInfo\CodeView\TypeRecord.h:

  // LF_BUILDINFO
  class BuildInfoRecord : public TypeRecord {
  public:
    BuildInfoRecord() = default;
    explicit BuildInfoRecord(TypeRecordKind Kind) : TypeRecord(Kind) {}
    BuildInfoRecord(ArrayRef<TypeIndex> ArgIndices)
        : TypeRecord(TypeRecordKind::BuildInfo),
          ArgIndices(ArgIndices.begin(), ArgIndices.end()) {}
  
    ArrayRef<TypeIndex> getArgs() const { return ArgIndices; }
  
    /// Indices of known build info arguments.
    enum BuildInfoArg {
      CurrentDirectory, ///< Absolute CWD path
      BuildTool,        ///< Absolute compiler path
      SourceFile,       ///< Path to main source file, relative or absolute
      TypeServerPDB,    ///< Absolute path of type server PDB (/Fd)
      CommandLine,      ///< Full canonical command line (maybe -cc1)
      MaxArgs
    };
  
    StaticVector<TypeIndex, MaxArgs> ArgIndices;
  };

Is the above substitution sufficient to move forward with a review?  Or would you like to see more wide-spread usage?

I suspect this container is going to be more attractive in target-specific code (as is my interest).  Is it appropriate for me to modify target-specific code to roll out a container like this?

> could fallible operations ala push_back return an llvm::error?

I implemented a subset of `checked_XXX` versions of fallible member functions this morning.  If this is an attractive feature, I can work on a complete set of them and add them to the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112175