Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2k
Warn when assembly output is in AT&T syntax (#4311)#8272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Warn when assembly output is in AT&T syntax (#4311)#8272
Conversation
Assembly documentation tooltips always display Intel syntax information, butwhen the actual assembly output is in AT&T syntax, this can be confusing dueto the reversed operand order (AT&T uses source, dest while Intel usesdest, source).This change adds a warning to the tooltip when the assembly output is NOT inIntel syntax, informing users that the documentation pertains to Intel syntax.The warning appears when:- The user has not enabled the Intel syntax filter (filters.intel is false)- The assembly output is therefore in AT&T syntaxThe detection uses filters.isSet('intel') rather than compiler.intelAsm,ensuring we check the actual output syntax (what the user selected) ratherthan just whether the compiler supports Intel syntax.Fixescompiler-explorer#4311refactor: move assembly-syntax type to frontend, remove from device-viewMove types/assembly-syntax.ts to static/assembly-syntax.ts since it's onlyused by frontend code (compiler.ts and device-view.ts), not backend.Per CONTRIBUTING.md, types/ directory is for shared types used by bothfrontend (static/) and backend (lib/). Since AssemblySyntax is frontend-only,it belongs in static/.Also removed syntax tracking from device-view entirely:- Device assembly (CUDA PTX, GPU, etc.) doesn't have Intel/AT&T variants- device-view was storing syntax as immutable state that never updated- This would cause incorrect tooltips if user toggled syntax after opening- Reverted device-view to match main branch (no syntax support)Changes:- Moved types/assembly-syntax.ts -> static/assembly-syntax.ts- Updated import paths in compiler.ts- Removed syntax field and imports from device-view.ts/.interfaces.tsThis means no unit tests are required per CONTRIBUTING.md guidelines(tests only required for server-side components in lib/).mattgodbolt commentedDec 9, 2025
Sorry it's taken so long to get to this: I've been away and then started a new job. Trying to work through the backlog now! |
mattgodbolt left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Basically LGTM! Sorry again for the delay. Just some comments/questions (exposing my lack of knowledge more than anything else)
| constCOMPILING_PLACEHOLDER='<Compiling...>'; | ||
| // Disable max line count only for the constructor. Turns out, it needs to do quite a lot of things | ||
| // Disable max line count only for the constructor. Turns out, it needs to do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
was there a reason to reformat this line?
| constcacheName=`asm/${instructionSet}/${opcode}`; | ||
| constcached=OpcodeCache.get(cacheName); | ||
| // Helper to add AT&T syntax warning to opcode data without mutating cache |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nice solution to this.
| if(cached.found)returncached.dataasAssemblyInstructionInfo; | ||
| if(cached.found){ | ||
| constcachedData=cached.dataasAssemblyInstructionInfo; | ||
| // Return a copy with warnings added based on current syntax mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not sure this comment adds anything - the function name is very descriptive. Justreturn addAttWEarningIfNeeded(cached.datat as AssmeblyInstructionIfo); just as you do below.
Assembly documentation tooltips always display Intel syntax information, but when the actual assembly output is in AT&T syntax, this can be confusing due to the reversed operand order (AT&T uses source, dest while Intel uses dest, source).
This change adds a warning to the tooltip when the assembly output is NOT in Intel syntax, informing users that the documentation pertains to Intel syntax.
The warning appears when:
The detection uses filters.isSet('intel') rather than compiler.intelAsm, ensuring we check the actual output syntax (what the user selected) rather than just whether the compiler supports Intel syntax.
Fixes#4311
refactor: move assembly-syntax type to frontend, remove from device-view
Move types/assembly-syntax.ts to static/assembly-syntax.ts since it's only used by frontend code (compiler.ts and device-view.ts), not backend.
Per CONTRIBUTING.md, types/ directory is for shared types used by both frontend (static/) and backend (lib/). Since AssemblySyntax is frontend-only, it belongs in static/.
Also removed syntax tracking from device-view entirely:
Changes:
This means no unit tests are required per CONTRIBUTING.md guidelines (tests only required for server-side components in lib/).