This is an archive of the discontinued LLVM Phabricator instance.

[Parse] Remove TimeTraceScope for "ParseTemplate"
ClosedPublic

Authored by MaskRay on Apr 14 2023, 10:36 PM.

Details

Summary

Fix https://github.com/llvm/llvm-project/issues/56554

#include "1.h"
#include "2.h"
int foo();

Suppose that 1.h ends with a template function. When parsing the function,
the ParseFunctionDefinition call after the TimeTraceScope object
may consume a r_brace token and lex the end of file (1.h), resulting
in an ExitFile event in SemaPPCallbacks::FileChanged. This event will call llvm::timeTraceProfilerEnd();,
which incorrectly ends "ParseTemplate" instead of "Source" (1.h).
Once 2.h has been fully parsed, the destructor of 1.h's TimeTraceScope object
will end "Source" (1.h).

This behavior erroneously extends the end of "Source" (1.h), which makes
"Source" (2.h) appear to be nested inside "Source" (1.h). This bug is difficult
to fix correctly in an elegant way, and we have two options: either end
"ParseTemplate" when ending "Source" (1.h), or delay the ExitFile event.

However, both approaches require complex code. For now, we can remove the
"ParseTemplate" TimeTraceScope. This can be re-added if properly repaired.

Diff Detail

Event Timeline

MaskRay created this revision.Apr 14 2023, 10:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2023, 10:36 PM
MaskRay requested review of this revision.Apr 14 2023, 10:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 14 2023, 10:36 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
anton-afanasyev accepted this revision.Apr 16 2023, 4:03 AM

Ok, let's remove it for now.

This revision is now accepted and ready to land.Apr 16 2023, 4:03 AM

Thanks @MaskRay, the fix is fine for me.

This revision was automatically updated to reflect the committed changes.