r252114 - [modules] If we're given a module file, via -fmodule-file=, for a module, but

[Richard Smith via cfe-commits]

On Thu, Feb 11, 2016 at 11:04 AM, Ben Langmuir <blangmuir at apple.com> wrote:
> 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.

Thank you.

> Ben
>
> On Jan 8, 2016, at 2:46 PM, Adrian Prantl via cfe-commits
> <cfe-commits at lists.llvm.org> wrote:
>
>
> On Jan 8, 2016, at 2:39 PM, Adrian Prantl <aprantl at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>