[PATCH] D82118: [clang][module] Improve incomplete-umbrella warning

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Okay so I think I traced down to the root problem: the diagnostic message prints out fine because `TextDiagnosticPrinter` uses `FullSourceLoc`, which actually embeds an appropriate `SourceManager` within it:

  TextDiag->emitDiagnostic(
      FullSourceLoc(Info.getLocation(), Info.getSourceManager()), Level,
      DiagMessageStream.str(), Info.getRanges(), Info.getFixItHints());

However, `FixItHint` is handled by a different path: a `FixItHint` only stores a trivial location (a `SourceRange`) which does not embed a `SourceManager`. And when a `FixItRewriter` constructs, it initializes a bunch of helper objects (Editor, Rewriter, Commit, etc.) using the current `SourceManager`. When trying to commit a fix-it hint, the helper objects use their (const ref to) `SourceManager`, which is used by the main compilation, to resolve the plain location in the `FixItHint`, which is generated in the new compiler instance for building a module. Therefore the location resolution is wrong.
To fix this, we can make `FixItHint` use `FullSourceLoc` instead, and also modify all relevant parties (`FixItRewriter`, `EditedSource`, `Rewriter`, `Commit`, ...) to use the embedded `SourceManager` instead of having a const ref to one initialized `SourceManager`.
My main concern is that I'm not very familiar with all the rewrite facilities, and changing `EditedSource` etc. might affect other users of the facilities.
What do you think? @arphaman @bruno @vsapsai


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82118