[PATCH] D95159: [ASTReader] Allow controlling separately whether validation should be disabled for a PCH vs a module file

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

The newly added test case clang/test/Index/preamble-reparse-changed-module.m is failing on the patch that I'm trying to commit, https://reviews.llvm.org/D93769

Actually the result of the test is a bit flaky, when I build the Debug version on Linux, running ninja check-clang does show the test fail, but when I do an identical build and in addition enabling llvm assertions, llvm-lit does not show this lit test failing.

I think the problem has something to do with the macro __FLT_EVAL_METHOD__.  The patch now treats this macro specially, more like __LINE__ a macro that can vary depending on where it is seen in the source code.

In my patch, __FLT_EVAL_MACRO__ is not expanded in -E mode, it just appears as its name in the preprocessed output.  The reason is, the actual value of the macro depends on the floating point settings at a particular source code location (depends on #pragma float_control settings).  The pragma are determined in Sema, and -E runs before Sema so the actual value isn't available.

This macro is the only reason i can think why this test is failing.

Can you help me understand what to do about this test case/about my patch/ so that i can get my patch committed? I don't understand what this test is doing. In the debugger it hits a failed assertion, looks like index out-of-range. here's a little massaged traceback
__assert_fail
 llvm::SmallVectorTemplateCommon unsigned long, void ::indexing operator
clang::ASTReader::ReadString
clang::ASTReader::ParseLanguageOptions
clang::ASTReader::ReadOptionsBlock
clang::ASTReader::ReadControlBlock
clang::ASTReader::ReadASTCore
clang::ASTReader::ReadAST
clang::CompilerInstance::findOrCompileModuleAndReadAST
clang::CompilerInstance::loadModule
clang::Preprocessor::LexAfterModuleImport

Thanks a lot


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95159/new/

https://reviews.llvm.org/D95159