FYI I changed this to early-exit again in r260563 to fix implicit builds. It’s not safe to keep reading if we haven’t told ReadOptionsBlock to expect failures, since it will itself early-exit, leaving the stream in the middle of a block. It’s also a performance advantage to early exit when doing implicit builds.
Ben > On Jan 8, 2016, at 2:46 PM, Adrian Prantl via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > >> >> On Jan 8, 2016, at 2:39 PM, Adrian Prantl <apra...@apple.com> wrote: >> >> Hi Richard, >> >> This change >>> @@ -2239,16 +2240,21 @@ ASTReader::ReadControlBlock(ModuleFile &F, >> [...] >>> - if (!DisableValidation && Result != Success && >>> - (Result != ConfigurationMismatch || >>> !AllowConfigurationMismatch)) >>> + if (DisableValidation || >>> + (AllowConfigurationMismatch && Result == >>> ConfigurationMismatch)) >>> + Result = Success; >>> + >>> + // If we've diagnosed a problem, we're done. >>> + if (Result != Success && >>> + isDiagnosedResult(Result, ClientLoadCapabilities)) >>> return Result; >> >> either causes or uncovers a bug: >> >>> CC=/Volumes/Data/llvm/_build.ninja.debug/bin/clang # r256948 >>> rm -rf cache && mkdir cache >>> rm -rf Test && mkdir Test >>> echo 'module Test { >>> umbrella header "Test.h" >>> }' >Test/module.modulemap >>> touch Test/Test.h >>> echo '#import <Test/Test.h>'>2.m >>> >>> clang -x objective-c -fmodules -fmodules-cache-path=cache -DA=0 -I. -c 2.m >>> -o 1.o >>> clang -x objective-c -fmodules -fmodules-cache-path=cache -Werror -DA=0 -I. >>> -c 2.m -o 2.o >>> >> >> >> After encountering a configuration mismatch or out-of-date error, we now >> continue instead of returning early and subsequently crash in >> >> ASTReader::ReadControlBlock() >> ASTReader::getInputFile() >> Cursor.JumpToBit(F.InputFileOffsets[ID-1]); >> >> I’ll keep digging deeper, but I thought you may have an intuition of what’s >> going on here. >> Is the behavior change intentional? From the commit message it sounds as if >> implicit module builds shouldn’t be affected. > > In my particular crash isDiagnosedResult returns false for an OutOfDate > result thus skipping the early return. > > -- adrian > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits