aaron.ballman added a comment. In https://reviews.llvm.org/D54450#1298319, @vmiklos wrote:
> Oh, and running the `check-clang-tools` target, this currently results in: > > Failing Tests (3): > Clang Tools :: modularize/ProblemsInconsistent.modularize > Clang Tools :: pp-trace/pp-trace-conditional.cpp > Clang Tools :: pp-trace/pp-trace-macro.cpp > > > Perhaps the pp-trace one just has to be updated for the new behavior. :-) Good catch -- I didn't run the clang-tools-extra tests. The pp-trace tests needed simple updating, but the modularize test has changed behavior and I'm not entirely certain of why. I've attached a file for the clang-tools-extra test changes: F7547470: preproc2.patch <https://reviews.llvm.org/F7547470> I am entirely unfamiliar with the "modularize" tool and there was a surprising change in behavior there, but I think the behavior is still okay (or, at least, no worse than the previous behavior which was already suspicious in that test case). ================ Comment at: lib/Lex/PPDirectives.cpp:553 } else { const SourceLocation CondBegin = CurPPLexer->getSourceLocation(); // Restore the value of LexingRawMode so that identifiers are ---------------- vmiklos wrote: > Is `CondBegin` still needed after your changes? Nope, good catch! ================ Comment at: lib/Lex/PPExpressions.cpp:852 DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective; return {false, DT.IncludedUndefinedIds}; } ---------------- vmiklos wrote: > The new `ExprRange` member is not initialized here, it seems. > `ExprRange` is default initialized in that case, so the default constructor is called as expected. https://reviews.llvm.org/D54450 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
