shafik added a comment. In D58743#1413249 <https://reviews.llvm.org/D58743#1413249>, @lebedev.ri wrote:
> Is there a test missing here? This origin of this fix is the LLDB expression parsing issue. We were not able to reduce to a test we could put in `ASTImpoterTest.cpp` but we have a LLDB test that exercises this issue and will pass once this fix is in place. See this differential https://reviews.llvm.org/D58790 ================ Comment at: lib/AST/ASTImporter.cpp:8284 + + if (!isBuiltin) { + // Include location of this file. ---------------- balazske wrote: > The `Cache` can be moved into this block and the block to `else if`. I am not sure I understand what you are asking. Do you mean just duplicate the const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache(); in both blocks? ================ Comment at: lib/AST/ASTImporter.cpp:8301 + ToID = ToSM.createFileID(Entry, ToIncludeLoc, + FromSLoc.getFile().getFileCharacteristic()); + } ---------------- balazske wrote: > Is it possible that in the `isBuiltin` true case this part of the code does > run (always) without assertion or other error and the result is always an > invalid `ToID`? (If yes the whole change is not needed, so I want to know > what was the reason for this change, was there any crash or bug.) This change was the result of a assert where evaluating an expression resulted in the built-in being imported and it would assert in `SourceManager` when when `Import(SourceLocation....)` attempted `getDecomposedLoc` at the next step. AFAICT the The Preprocessor sets it up the buffer [here](https://github.com/llvm-mirror/clang/blob/master/lib/Lex/Preprocessor.cpp#L552) and then creates the `FileID` for in the `SourceManager` and copying over the buffer should be the correct thing to do since that seems to be sufficient. I also while testing verified that the `BufferIndetifer()` did indeed equal `"<built-in>"` similar to [this check](https://github.com/llvm-mirror/clang/blob/master/lib/Basic/SourceManager.cpp#L2009) in `SourceManager`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58743/new/ https://reviews.llvm.org/D58743 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
