This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Use all query-driver arguments in cache key

Authored by nridge on Dec 4 2022, 10:54 AM.



Rework the cache key construction to consider all the arguments used when invoking the query driver.

This also reworks handling for -isysroot

And adds support for passing through -specs flags to the query driver

Diff Detail

Event Timeline

ehntoo created this revision.Dec 4 2022, 10:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 4 2022, 10:54 AM
Herald added a subscriber: arphaman. · View Herald Transcript
ehntoo requested review of this revision.Dec 4 2022, 10:54 AM

thanks a lot for taking a look at this!


can we introduce a struct instead?

struct DriverArgs {
  std::string Driver;
  bool StandardIncludes = true;
  bool StandardCXXIncludes = true;
  bool BuiltinIncludes = true;
  llvm::StringRef Lang;
  llvm::StringRef Sysroot;

  DriverArgs(const tooling::CompileCommand &Cmd); // Traverses the Cmd and infers the bits.
  llvm::SmallVector<llvm::StringRef> render() const; // we can use canonical versions.

we can also implement hashing based on the struct now and also pass it around.


i don't think we gain much by having these 3 different types now, especially considering an arg might need to be placed in multiple categories (-isysroot in both TwoPartArgs and ArgPrefixes).

let's rather have a check for each related bit of struct inside the for loop, e.g. something like:

for(...) {
  // Infer Lang
  if(Arg.startswith("-x")) {
    if (Arg == "-x" && I + 1 < E) Lang = Args[++I];
    else Lang = Arg.drop_front(2).trim();

  // Infer Sysroot

can we add --specs related changes in a follow up patch instead?

@ehntoo hi! are you planning to update this patch to address Kadir's comments?

This was superseded by D146941.

nridge commandeered this revision.Sep 9 2023, 12:49 AM
nridge abandoned this revision.
nridge edited reviewers, added: ehntoo; removed: nridge.