malcolm.parsons added inline comments.
================ Comment at: include/clang/AST/TypeLoc.h:533 + } else { + BuiltinRange.setBegin(std::min(Range.getBegin(), BuiltinRange.getBegin())); + BuiltinRange.setEnd(std::max(Range.getEnd(), BuiltinRange.getEnd())); ---------------- aaron.ballman wrote: > malcolm.parsons wrote: > > aaron.ballman wrote: > > > malcolm.parsons wrote: > > > > I suspect that using `min` and `max` on `SourceLocation`s is only valid > > > > if both locations are in the same file. > > > > > > > > Doing this > > > > long.h: > > > > ``` > > > > long > > > > ``` > > > > unsigned.cpp: > > > > ``` > > > > unsigned > > > > #include "long.h" > > > > i; > > > > ``` > > > > > > > > causes > > > > clang-query> match typeLoc() > > > > ... > > > > clang-query: llvm/tools/clang/lib/Frontend/DiagnosticRenderer.cpp:273: > > > > clang::SourceLocation retrieveMacroLocation(clang::SourceLocation, > > > > clang::FileID, clang::FileID, const > > > > llvm::SmallVectorImpl<clang::FileID>&, bool, const > > > > clang::SourceManager*): Assertion `SM->getFileID(Loc) == MacroFileID' > > > > failed. > > > > > > > > Is there a better way to combine `SourceRange`s, or should the final > > > > range be accumulated in DeclSpec.cpp? > > > Hmm, that's a good point, but I'm not aware of a better utility. Perhaps > > > @rsmith knows of one? > > I get the same assertion failure from a clang-query without my changes if I > > match a `CompoundStmt` that starts and ends in different files. > > So I don't think it's my fault. > > It's also a very rare case. > > OK to commit? > Thank you for checking. I think you are okay to commit, we can correct the > bug in another patch. However, if you wouldn't mind filing a bug in bugzilla > so that we don't lose this information, that would be great. PR30765. Repository: rL LLVM https://reviews.llvm.org/D25363 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits