Page MenuHomePhabricator

Do not validate pch when -fno-validate-pch is set
ClosedPublic

Authored by yaxunl on Aug 30 2016, 1:45 PM.

Details

Summary

There is a bug causing pch to be validated even though -fno-validate-pch is set. This patch fixes it.

ASTReader relies on ASTReaderListener to initialize SuggestedPredefines, which is required for compilations using PCH. Before this change, PCHValidator is the default ASTReaderListener. After this change, when -fno-validate-pch is set, PCHValidator is disabled, but we need a replacement ASTReaderListener to initialize SuggestedPredefines. Class SimpleASTReaderListener is implemented for this purpose.

This change only affects -fno-validate-pch. There is no functional change if -fno-validate-pch is not set.

If -fno-validate-pch is not set, conflicts in predefined macros between pch and current compiler instance causes error.

If -fno-validate-pch is set, predefine macros in current compiler override those in pch so that compilation can continue.

Diff Detail

Event Timeline

yaxunl updated this revision to Diff 69749.Aug 30 2016, 1:45 PM
yaxunl retitled this revision from to Do not validate pch when -fno-validate-pch is set.
yaxunl updated this object.
yaxunl added reviewers: rsmith, Anastasia.
yaxunl added a subscriber: cfe-commits.
yaxunl added a comment.Sep 2 2016, 9:33 AM

The bug is due to PCHValidator is added as Listener, which causes pch is validated even though DisableValidation is true (when -fno-validate-pch is on). The fix is not add PCHValidator as listener when DisableValidation is true.

The bug is annoying for using pch. When pch is generated with default optimization, the generated pch cannot be used to compile program with -O0.

Tom/Alexey, can you help review the code? Thanks.

bader edited edge metadata.Sep 4 2016, 4:35 AM

Could you a regression test, please?

yaxunl updated this revision to Diff 70569.Sep 7 2016, 11:16 AM
yaxunl updated this object.
yaxunl edited edge metadata.

Implemented a simple AST reader listener to replace PCHValidator. Added a test.

yaxunl updated this object.Sep 7 2016, 11:17 AM
Anastasia accepted this revision.Sep 7 2016, 11:35 AM
Anastasia edited edge metadata.

LGTM!

This revision is now accepted and ready to land.Sep 7 2016, 11:35 AM
rsmith accepted this revision.Sep 7 2016, 11:40 AM
rsmith edited edge metadata.

It's pretty gross that we're computing SuggestedPredefines in the ASTReaderListener, but sure, this seems OK for now.

bader accepted this revision.Sep 7 2016, 11:45 AM
bader edited edge metadata.
This revision was automatically updated to reflect the committed changes.