128505 – Never include *Inlines.h files in interface headers, and never include *Inlines.h when you could include Operations.h instead

RESOLVED FIXED 128505

Never include *Inlines.h files in interface headers, and never include *Inlines.h when you could include Operations.h instead

https://bugs.webkit.org/show_bug.cgi?id=128505

Summary Never include *Inlines.h files in interface headers, and never include *Inlin...

Filip Pizlo

Reported 2014-02-09 14:26:13 PST

Patch forthcoming.

Attachments
the patch (41.19 KB, patch) 2014-02-09 14:35 PST, Filip Pizlo	mhahnenberg: review+	Details Formatted Diff Diff
the patch (72.07 KB, patch) 2014-02-09 15:15 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (81.27 KB, patch) 2014-02-09 15:34 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (76.79 KB, patch) 2014-02-09 15:40 PST, Filip Pizlo	oliver: review+	Details Formatted Diff Diff
the patch (85.38 KB, patch) 2014-02-09 16:40 PST, Filip Pizlo	no flags	Details Formatted Diff Diff
the patch (87.79 KB, patch) 2014-02-09 16:52 PST, Filip Pizlo	fpizlo: review+	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Filip Pizlo

Comment 1 2014-02-09 14:35:36 PST

Created attachment 223648 [details] the patch

WebKit Commit Bot

Comment 2 2014-02-09 14:36:45 PST

Attachment 223648 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:33: "Operations.h" already included at Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:31 [build/include] [4] Total errors found: 1 in 72 files If any of these errors are false positives, please file a bug against check-webkit-style.

Mark Hahnenberg

Comment 3 2014-02-09 14:36:46 PST

Comment on attachment 223648 [details] the patch r=me. Is there something we might be able to add to the style checker to enforce these rules, at least for JSC?

Mark Hahnenberg

Comment 4 2014-02-09 14:37:05 PST

(In reply to comment #2) > Attachment 223648 [details] did not pass style-queue: > > > ERROR: Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:33: "Operations.h" already included at Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:31 [build/include] [4] > Total errors found: 1 in 72 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. Oh but you should also fix this :-)

Filip Pizlo

Comment 5 2014-02-09 14:37:42 PST

(In reply to comment #4) > (In reply to comment #2) > > Attachment 223648 [details] [details] did not pass style-queue: > > > > > > ERROR: Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:33: "Operations.h" already included at Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:31 [build/include] [4] > > Total errors found: 1 in 72 files > > > > > > If any of these errors are false positives, please file a bug against check-webkit-style. > > Oh but you should also fix this :-) Yeah :-)

Filip Pizlo

Comment 6 2014-02-09 14:38:22 PST

(In reply to comment #3) > (From update of attachment 223648 [details]) > r=me. Is there something we might be able to add to the style checker to enforce these rules, at least for JSC? I hope so! Btw, this would have to be a JSC-only rule. In WebCore, Operations.h is included by JSBindings.h or something so that you don't have to include it everywhere. That's fine since none of JSC's *Inlines.h would ever include any WebCore headers.

Filip Pizlo

Comment 7 2014-02-09 15:15:57 PST

Created attachment 223651 [details] the patch More stuff. Same basic idea as the last patch.

WebKit Commit Bot

Comment 8 2014-02-09 15:17:33 PST

Attachment 223651 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGCompilationMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCriticalEdgeBreakingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableEventStream.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAbstractValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGWorklist.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNodeFlags.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredStructureChains.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPredictionInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:31: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCFGSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLongLivedState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCommonData.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFailedFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGClobberSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGMinifiedNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDominators.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLoopPreHeaderCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITCode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAvailability.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableEvent.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredIdentifiers.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushFormat.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitJumpPlaceholder.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBinarySwitch.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCapabilities.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStoreBarrierElisionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCSEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNaturalLoops.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGValidate.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCPSRethreadingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitPreparation.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGUnificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAbstractHeap.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDCEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGValueSource.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJumpReplacement.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushedAt.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCFAPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGTypeCheckHoistingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableAccessDataDump.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSREntry.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExit.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGClobberize.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVirtualRegisterAllocationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGToFTLDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPlan.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitBase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDisassembler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLazyJSValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGInvalidationPointInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGUseKind.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGEdge.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAtTailAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGThunks.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] Total errors found: 91 in 121 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 9 2014-02-09 15:21:10 PST

(In reply to comment #8) > Attachment 223651 [details] did not pass style-queue: > > > ERROR: Source/JavaScriptCore/dfg/DFGCompilationMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGCriticalEdgeBreakingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGVariableEventStream.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGAbstractValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGWorklist.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGNodeFlags.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGDesiredStructureChains.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGPredictionInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:31: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGCFGSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGLongLivedState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGCommonData.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGFailedFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGClobberSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGMinifiedNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGDominators.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGLoopPreHeaderCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGJITCode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGAvailability.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGVariableEvent.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGDesiredIdentifiers.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGFlushFormat.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSRExitJumpPlaceholder.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGBinarySwitch.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGCapabilities.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGStoreBarrierElisionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGCSEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGNaturalLoops.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGValidate.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGCPSRethreadingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSRExitPreparation.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGUnificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGAbstractHeap.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGDCEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGValueSource.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGJumpReplacement.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGFlushedAt.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGCFAPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGTypeCheckHoistingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGVariableAccessDataDump.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSREntry.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSRExit.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGFlushLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGClobberize.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGVirtualRegisterAllocationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGToFTLDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGPlan.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSRExitBase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGDisassembler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGLazyJSValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGInvalidationPointInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGUseKind.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGEdge.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGAtTailAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > ERROR: Source/JavaScriptCore/dfg/DFGThunks.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > Total errors found: 91 in 121 files > > > If any of these errors are false positives, please file a bug against check-webkit-style. No, these aren't real errors. EFL is using -Wundefined-inline. I think it's a good warning flag, and clang added it sometime recently so clang-based builds might end up using it, too. To make this flag work, files should never unconditionally include the cpp module's corresponding interface header but then conditionally incl *Inlines.h or Operations.h. The best way to make this work is to either never conditionally include headers, or to wrap the .cpp file's condition around the whole file excluding config.h. I think that the latter is more sensible.

Filip Pizlo

Comment 10 2014-02-09 15:34:59 PST

Created attachment 223654 [details] the patch

Filip Pizlo

Comment 11 2014-02-09 15:40:18 PST

Created attachment 223655 [details] the patch

WebKit Commit Bot

Comment 12 2014-02-09 15:43:08 PST

Attachment 223655 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGCompilationMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCriticalEdgeBreakingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableEventStream.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAbstractValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGWorklist.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNodeFlags.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredStructureChains.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPredictionInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:31: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCFGSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLongLivedState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCommonData.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFailedFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGClobberSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGMinifiedNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDominators.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLoopPreHeaderCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITCode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAvailability.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableEvent.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredIdentifiers.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushFormat.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitJumpPlaceholder.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBinarySwitch.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCapabilities.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStoreBarrierElisionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCSEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNaturalLoops.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGValidate.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCPSRethreadingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitPreparation.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGUnificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAbstractHeap.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDCEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGValueSource.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJumpReplacement.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushedAt.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCFAPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGTypeCheckHoistingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableAccessDataDump.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSREntry.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExit.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGClobberize.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVirtualRegisterAllocationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGToFTLDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPlan.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitBase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDisassembler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLazyJSValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGInvalidationPointInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGUseKind.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGEdge.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAtTailAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGThunks.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] Total errors found: 92 in 139 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sergio Correia (qrwteyrutiyoup)

Comment 13 2014-02-09 16:24:37 PST

(In reply to comment #9) > No, these aren't real errors. EFL is using -Wundefined-inline. I think it's a good warning flag, and clang added it sometime recently so clang-based builds might end up using it, too. To make this flag work, files should never unconditionally include the cpp module's corresponding interface header but then conditionally incl *Inlines.h or Operations.h. The best way to make this work is to either never conditionally include headers, or to wrap the .cpp file's condition around the whole file excluding config.h. I think that the latter is more sensible. You might want to remove the inline keyword from "inline bool isHostFunction() const" (JSFunction.h), like Oliver did in the r163195 odissey (and got rolled out in r163225).

Filip Pizlo

Comment 14 2014-02-09 16:39:45 PST

(In reply to comment #13) > (In reply to comment #9) > > No, these aren't real errors. EFL is using -Wundefined-inline. I think it's a good warning flag, and clang added it sometime recently so clang-based builds might end up using it, too. To make this flag work, files should never unconditionally include the cpp module's corresponding interface header but then conditionally incl *Inlines.h or Operations.h. The best way to make this work is to either never conditionally include headers, or to wrap the .cpp file's condition around the whole file excluding config.h. I think that the latter is more sensible. > > You might want to remove the inline keyword from "inline bool isHostFunction() const" (JSFunction.h), like Oliver did in the r163195 odissey (and got rolled out in r163225). Hmmm. I kind of like the idea of having that function be inline. And I like the basic gist of what the warning is trying to do.

Filip Pizlo

Comment 15 2014-02-09 16:40:14 PST

Created attachment 223656 [details] the patch

WebKit Commit Bot

Comment 16 2014-02-09 16:42:43 PST

Attachment 223656 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGCompilationMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCriticalEdgeBreakingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableEventStream.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAbstractValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGWorklist.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNodeFlags.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredStructureChains.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPredictionInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:31: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCFGSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLongLivedState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCommonData.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFailedFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGClobberSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGMinifiedNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDominators.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLoopPreHeaderCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITCode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAvailability.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableEvent.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredIdentifiers.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushFormat.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/JITOperationsMSVC64.cpp:32: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/TempRegisterSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitJumpPlaceholder.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBinarySwitch.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCapabilities.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStoreBarrierElisionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCSEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNaturalLoops.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGValidate.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCPSRethreadingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitPreparation.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGUnificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAbstractHeap.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDCEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGValueSource.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJumpReplacement.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushedAt.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCFAPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGTypeCheckHoistingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableAccessDataDump.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/JITDisassembler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSREntry.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExit.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGClobberize.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/JITStubRoutine.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVirtualRegisterAllocationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGToFTLDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPlan.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitBase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDisassembler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLazyJSValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGInvalidationPointInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGUseKind.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGEdge.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAtTailAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGThunks.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] Total errors found: 97 in 156 files If any of these errors are false positives, please file a bug against check-webkit-style.

Sergio Correia (qrwteyrutiyoup)

Comment 17 2014-02-09 16:46:28 PST

(In reply to comment #14) > (In reply to comment #13) > > (In reply to comment #9) > > > No, these aren't real errors. EFL is using -Wundefined-inline. I think it's a good warning flag, and clang added it sometime recently so clang-based builds might end up using it, too. To make this flag work, files should never unconditionally include the cpp module's corresponding interface header but then conditionally incl *Inlines.h or Operations.h. The best way to make this work is to either never conditionally include headers, or to wrap the .cpp file's condition around the whole file excluding config.h. I think that the latter is more sensible. > > > > You might want to remove the inline keyword from "inline bool isHostFunction() const" (JSFunction.h), like Oliver did in the r163195 odissey (and got rolled out in r163225). > > Hmmm. I kind of like the idea of having that function be inline. And I like the basic gist of what the warning is trying to do. It would still be marked inline in its definition, in Executable.h. Isn't that enough for suggesting the compiler you'd like that function to be inline?

Filip Pizlo

Comment 18 2014-02-09 16:48:12 PST

(In reply to comment #17) > (In reply to comment #14) > > (In reply to comment #13) > > > (In reply to comment #9) > > > > No, these aren't real errors. EFL is using -Wundefined-inline. I think it's a good warning flag, and clang added it sometime recently so clang-based builds might end up using it, too. To make this flag work, files should never unconditionally include the cpp module's corresponding interface header but then conditionally incl *Inlines.h or Operations.h. The best way to make this work is to either never conditionally include headers, or to wrap the .cpp file's condition around the whole file excluding config.h. I think that the latter is more sensible. > > > > > > You might want to remove the inline keyword from "inline bool isHostFunction() const" (JSFunction.h), like Oliver did in the r163195 odissey (and got rolled out in r163225). > > > > Hmmm. I kind of like the idea of having that function be inline. And I like the basic gist of what the warning is trying to do. > > It would still be marked inline in its definition, in Executable.h. Isn't that enough for suggesting the compiler you'd like that function to be inline? You're right. But is it clear that we *don't* want this warning? If we don't want it, then isn't the solution just to remove that warning from the EFL build?

Filip Pizlo

Comment 19 2014-02-09 16:52:44 PST

Created attachment 223658 [details] the patch

WebKit Commit Bot

Comment 20 2014-02-09 16:54:16 PST

Attachment 223658 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/dfg/DFGCompilationMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCriticalEdgeBreakingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableEventStream.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAbstractValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGWorklist.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNodeFlags.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredStructureChains.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGResurrectionForValidationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPredictionInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBlockInsertionSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp:31: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArgumentsSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCFGSimplificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLICMPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLongLivedState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRAvailabilityAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStrengthReductionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCommonData.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFailedFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGClobberSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGMinifiedNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDominators.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSSALoweringPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLoopPreHeaderCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJITCode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAvailability.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGTierUpCheckInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableEvent.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredIdentifiers.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushFormat.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/JITOperationsMSVC64.cpp:32: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/TempRegisterSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitJumpPlaceholder.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBinarySwitch.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSREntrypointCreationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/heap/JITStubRoutineSet.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCapabilities.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStoreBarrierElisionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCSEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGNaturalLoops.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGValidate.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCPSRethreadingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitPreparation.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGUnificationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAbstractHeap.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDCEPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArithMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGValueSource.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGJumpReplacement.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushedAt.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCFAPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFinalizer.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGTypeCheckHoistingPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVariableAccessDataDump.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGArrayMode.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/JITDisassembler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSREntry.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExit.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGFlushLivenessAnalysisPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGClobberize.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/jit/JITStubRoutine.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGVirtualRegisterAllocationPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGToFTLDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompiler32_64.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGPlan.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitBase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDisassembler.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGBasicBlock.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGLazyJSValue.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGDesiredWatchpoints.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGInvalidationPointInjectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGToFTLForOSREntryDeferredCompilationCallback.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGUseKind.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGWatchpointCollectionPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGEdge.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGAtTailAbstractState.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] ERROR: Source/JavaScriptCore/dfg/DFGThunks.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] Total errors found: 97 in 162 files If any of these errors are false positives, please file a bug against check-webkit-style.

Filip Pizlo

Comment 21 2014-02-09 18:47:12 PST

Comment on attachment 223658 [details] the patch This was already reviewed by Mark and Oliver.

Filip Pizlo

Comment 22 2014-02-09 18:47:43 PST

Landed in http://trac.webkit.org/changeset/163760

Csaba Osztrogonác

Comment 23 2014-02-11 09:27:29 PST

Comment on attachment 223658 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=223658&action=review > Source/JavaScriptCore/dfg/DFGDisassembler.h:-119 > -#else // ENABLE(DISASSEMBLER) > - > -class Disassembler { > - WTF_MAKE_FAST_ALLOCATED; > -public: > - Disassembler(Graph&) { } > - > - void setStartOfCode(MacroAssembler::Label) { } > - void setForBlockIndex(BlockIndex, MacroAssembler::Label) { } > - void setForNode(Node*, MacroAssembler::Label) { } > - void setEndOfMainPath(MacroAssembler::Label) { } > - void setEndOfCode(MacroAssembler::Label) { } > - > - void dump(LinkBuffer&) { } > - void reportToProfiler(Profiler::Compilation*, LinkBuffer&) { } > -}; > - > -#endif // ENABLE(DISASSEMBLER) How is this change related to "Never include *Inlines.h files in interface headers, and never include *Inlines.h when you could include Operations.h instead" topic?

Filip Pizlo

Comment 24 2014-02-11 09:31:49 PST

(In reply to comment #23) > (From update of attachment 223658 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=223658&action=review > > > Source/JavaScriptCore/dfg/DFGDisassembler.h:-119 > > -#else // ENABLE(DISASSEMBLER) > > - > > -class Disassembler { > > - WTF_MAKE_FAST_ALLOCATED; > > -public: > > - Disassembler(Graph&) { } > > - > > - void setStartOfCode(MacroAssembler::Label) { } > > - void setForBlockIndex(BlockIndex, MacroAssembler::Label) { } > > - void setForNode(Node*, MacroAssembler::Label) { } > > - void setEndOfMainPath(MacroAssembler::Label) { } > > - void setEndOfCode(MacroAssembler::Label) { } > > - > > - void dump(LinkBuffer&) { } > > - void reportToProfiler(Profiler::Compilation*, LinkBuffer&) { } > > -}; > > - > > -#endif // ENABLE(DISASSEMBLER) > > How is this change related to "Never include *Inlines.h files in interface headers, and never include *Inlines.h when you could include Operations.h instead" topic? It was just obviously wrong code.

Csaba Osztrogonác

Comment 25 2014-02-11 09:34:12 PST

Comment on attachment 223658 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=223658&action=review > Source/JavaScriptCore/dfg/DFGPhase.cpp:31 > #include "config.h" > -#include "DFGPhase.h" > > #if ENABLE(DFG_JIT) > > +#include "DFGPhase.h" > + ERROR: Source/JavaScriptCore/dfg/DFGPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] Why do we need zillion of this kind of coding style violations? For example the whole DFGPhase.h is guarded by ENABLE(DFG_JIT). Why do we need guard it again in this cpp?

Filip Pizlo

Comment 26 2014-02-11 09:43:16 PST

(In reply to comment #25) > (From update of attachment 223658 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=223658&action=review > > > Source/JavaScriptCore/dfg/DFGPhase.cpp:31 > > #include "config.h" > > -#include "DFGPhase.h" > > > > #if ENABLE(DFG_JIT) > > > > +#include "DFGPhase.h" > > + > > ERROR: Source/JavaScriptCore/dfg/DFGPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > > Why do we need zillion of this kind of coding style violations? > For example the whole DFGPhase.h is guarded by ENABLE(DFG_JIT). > Why do we need guard it again in this cpp? EFL build sometimes gets unhappy if you include a header that declares an inline function but fails to include the header that defines it. Generally, interface headers such as DFGPhase.h may declare inline functions. Even though that's not the case here, I figured it would be better to have a unified story for this. In some places we were doing: #include "config.h" #include "Blah.h" #if ENABLE(...) And in other places we were doing: #include "config.h" #if ENABLE(...) #include "Blah.h" I just switched all of the code I knew about to using the second style. That fixed the EFL builds and made everything more consistent.

Csaba Osztrogonác

Comment 27 2014-02-12 02:01:51 PST

(In reply to comment #24) > (In reply to comment #23) > > (From update of attachment 223658 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=223658&action=review > > > > > Source/JavaScriptCore/dfg/DFGDisassembler.h:-119 > > > -#else // ENABLE(DISASSEMBLER) > > > - > > > -class Disassembler { > > > - WTF_MAKE_FAST_ALLOCATED; > > > -public: > > > - Disassembler(Graph&) { } > > > - > > > - void setStartOfCode(MacroAssembler::Label) { } > > > - void setForBlockIndex(BlockIndex, MacroAssembler::Label) { } > > > - void setForNode(Node*, MacroAssembler::Label) { } > > > - void setEndOfMainPath(MacroAssembler::Label) { } > > > - void setEndOfCode(MacroAssembler::Label) { } > > > - > > > - void dump(LinkBuffer&) { } > > > - void reportToProfiler(Profiler::Compilation*, LinkBuffer&) { } > > > -}; > > > - > > > -#endif // ENABLE(DISASSEMBLER) > > > > How is this change related to "Never include *Inlines.h files in interface headers, and never include *Inlines.h when you could include Operations.h instead" topic? > > It was just obviously wrong code. The question wasn't here if it is obviously wrong or not. It is unconventional to push random fixes in absolutely unrelated bug reports without changelog entry, without any comment and with zillion coding style violation.

Csaba Osztrogonác

Comment 28 2014-02-12 02:06:12 PST

(In reply to comment #26) > (In reply to comment #25) > > (From update of attachment 223658 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=223658&action=review > > > > > Source/JavaScriptCore/dfg/DFGPhase.cpp:31 > > > #include "config.h" > > > -#include "DFGPhase.h" > > > > > > #if ENABLE(DFG_JIT) > > > > > > +#include "DFGPhase.h" > > > + > > > > ERROR: Source/JavaScriptCore/dfg/DFGPhase.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] > > > > Why do we need zillion of this kind of coding style violations? > > For example the whole DFGPhase.h is guarded by ENABLE(DFG_JIT). > > Why do we need guard it again in this cpp? > > EFL build sometimes gets unhappy if you include a header that declares an inline function but fails to include the header that defines it. Generally, interface headers such as DFGPhase.h may declare inline functions. Even though that's not the case here, I figured it would be better to have a unified story for this. In some places we were doing: > > #include "config.h" > #include "Blah.h" > > #if ENABLE(...) > > And in other places we were doing: > > #include "config.h" > > #if ENABLE(...) > > #include "Blah.h" > > I just switched all of the code I knew about to using the second style. That fixed the EFL builds and made everything more consistent. I don't think at all if it is the proper way to fix the issue you mentioned. Let me investigate the real problem. I'm going to find a better fix than adding more and more coding style violations and many redundant ENABLE macros.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware All

OS All

Product WebKit

Component JavaScriptCore

Assignee

Filip Pizlo

Reported

2014-02-09 14:26 PST

Modified

2014-02-17 04:48 PST History

CC List

12 users Show

URL

Keywords

Depends on

128900

Blocks

128507 128291

Dependencies

tree graph